From add5f757870cfb9a371169290533b1cd404845b3 Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Tue, 20 Oct 2026 07:27:15 -0700 Subject: [PATCH] =?UTF-8?q?Overlay=20Mapper:=20rendered=20status=20test,?= =?UTF-8?q?=20wiring=20lint,=20capture=E2=86=92save=20end=20to=20end?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hardware check found a declined save that nothing showed and nothing logged; the engine's tests could not see it because they assert on the model. Three layers now cover the overlay seam: MapperStatusLine (extracted from OverlayMapperSection) is rendered for real and must draw a message, draw errors in red and draw nothing for nil; a lint keeps the overlay reading viewModel.statusMessage and keeps save() from returning without a status and a log line (it immediately found the unlogged app-specific failure path, fixed here); and MediaKeyCaptureToSaveTests drives a synthesized system-defined Mute/volume/brightness/play event through KeyboardCapture (now a pure `mediaKeyPress(from:)` seam) → Mapper → save → generated config against the real bridge. SwiftUI publishes no accessibility tree in-process, so the rendered test rasterizes rather than reading text; appearance stays with the opt-in snapshot target. Co-Authored-By: Claude Fable 5.1 --- .../KeyboardCapture/KeyboardCapture.swift | 42 +++++------ .../MapperViewModel+RuleManagement.swift | 1 + .../UI/Overlay/MapperStatusLine.swift | 24 +++++++ .../UI/Overlay/OverlayMapperSection.swift | 13 +--- .../Lint/OverlayMapperStatusLintTests.swift | 33 +++++++++ .../UI/MapperStatusLineTests.swift | 51 +++++++++++++ .../UI/MediaKeyCaptureToSaveTests.swift | 72 +++++++++++++++++++ docs/architecture/rules-admission-review.md | 9 +++ 8 files changed, 210 insertions(+), 35 deletions(-) create mode 100644 Sources/KeyPathAppKit/UI/Overlay/MapperStatusLine.swift create mode 100644 Tests/KeyPathTests/Lint/OverlayMapperStatusLintTests.swift create mode 100644 Tests/KeyPathTests/UI/MapperStatusLineTests.swift create mode 100644 Tests/KeyPathTests/UI/MediaKeyCaptureToSaveTests.swift diff --git a/Sources/KeyPathAppKit/Services/KeyboardCapture/KeyboardCapture.swift b/Sources/KeyPathAppKit/Services/KeyboardCapture/KeyboardCapture.swift index b1899340f..ada758461 100644 --- a/Sources/KeyPathAppKit/Services/KeyboardCapture/KeyboardCapture.swift +++ b/Sources/KeyPathAppKit/Services/KeyboardCapture/KeyboardCapture.swift @@ -544,36 +544,32 @@ public class KeyboardCapture { /// Handle media key events (volume, brightness, play/pause, etc.) /// These come through NSEvent.systemDefined, not regular keyDown events. - private func handleMediaKeyEvent(_ event: NSEvent) { - guard isCapturing else { return } - guard event.subtype.rawValue == 8 else { return } // 8 = media key subtype - - // Media key data is encoded in data1: - // - bits 16-23: key code (NX_KEYTYPE_*) - // - bit 8: key state (0 = down, 1 = up) - // - bits 0-7: repeat count + /// The key press a system-defined (subtype 8) media event captures, or nil + /// for a key-up or a key KeyPath does not name. Pure, so the capture → save + /// path can be tested from a synthesized event. + /// + /// data1 carries the `NX_KEYTYPE_*` code in bits 16–23, the key state in + /// bit 8 (0 down, 1 up) and the repeat count in bits 0–7. The key code is + /// offset by 1000 so it cannot collide with a keyboard key code. + static func mediaKeyPress(from event: NSEvent, at now: Date = Date()) -> KeyPress? { + guard event.subtype.rawValue == 8 else { return nil } let data1 = event.data1 let mediaKeyCode = (data1 & 0x00FF_0000) >> 16 - let keyState = (data1 & 0x0000_0100) >> 8 - let isKeyDown = keyState == 0 - - guard isKeyDown else { return } // Only capture key down events - - guard let keyName = mediaKeyCodeToString(Int(mediaKeyCode)) else { return } + guard (data1 & 0x0000_0100) >> 8 == 0 else { return nil } + guard let keyName = mediaKeyCodeToString(Int(mediaKeyCode)) else { return nil } + return KeyPress(baseKey: keyName, modifiers: [], timestamp: now, keyCode: Int64(mediaKeyCode) + 1000) + } + private func handleMediaKeyEvent(_ event: NSEvent) { + guard isCapturing else { return } let now = Date() - AppLogger.shared.log("🎹 [KeyboardCapture] mediaKey: \(keyName) code=\(mediaKeyCode)") + guard let keyPress = Self.mediaKeyPress(from: event, at: now) else { return } + let keyName = keyPress.baseKey + AppLogger.shared.log("🎹 [KeyboardCapture] mediaKey: \(keyName) code=\(keyPress.keyCode - 1000)") anyEventSeen = true noKeyBreadcrumbTimer?.invalidate() noKeyBreadcrumbTimer = nil - let keyPress = KeyPress( - baseKey: keyName, - modifiers: [], - timestamp: now, - keyCode: Int64(mediaKeyCode) + 1000 // Offset to avoid collision with regular keyCodes - ) - // De-dup identical events if let last = lastCapturedKey, let lastAt = lastCaptureAt { if last.baseKey == keyPress.baseKey, @@ -600,7 +596,7 @@ public class KeyboardCapture { } /// Convert NX_KEYTYPE media key code to a string name - private func mediaKeyCodeToString(_ keyCode: Int) -> String? { + static func mediaKeyCodeToString(_ keyCode: Int) -> String? { // NX_KEYTYPE values from IOKit/hidsystem/ev_keymap.h let mediaKeyMap: [Int: String] = [ 0: "volumeup", // NX_KEYTYPE_SOUND_UP diff --git a/Sources/KeyPathAppKit/UI/Mapper/MapperViewModel+RuleManagement.swift b/Sources/KeyPathAppKit/UI/Mapper/MapperViewModel+RuleManagement.swift index 17c867b2d..874e7c18e 100644 --- a/Sources/KeyPathAppKit/UI/Mapper/MapperViewModel+RuleManagement.swift +++ b/Sources/KeyPathAppKit/UI/Mapper/MapperViewModel+RuleManagement.swift @@ -60,6 +60,7 @@ extension MapperViewModel { SoundPlayer.shared.playSuccessSound() AppLogger.shared.log("✅ [MapperViewModel] Saved app-specific mapping: \(inputSeq.displayString) → \(outputSeq.displayString) [only in \(appCondition.displayName)]") } else { + AppLogger.shared.log("🚫 [MapperViewModel] App-specific save failed: \(inputKanata) → \(outputKanata) for \(appCondition.bundleIdentifier)") statusMessage = "Failed to save app-specific rule" statusIsError = true } diff --git a/Sources/KeyPathAppKit/UI/Overlay/MapperStatusLine.swift b/Sources/KeyPathAppKit/UI/Overlay/MapperStatusLine.swift new file mode 100644 index 000000000..3fb604ae5 --- /dev/null +++ b/Sources/KeyPathAppKit/UI/Overlay/MapperStatusLine.swift @@ -0,0 +1,24 @@ +import SwiftUI + +/// The overlay Mapper's one line of status. The overlay saves on capture and +/// has no Save button, so a refusal, a declined save or "✓ Saved" must be said +/// here; without it a declined mapping just seems to vanish. +struct MapperStatusLine: View { + let message: String? + let isError: Bool + + var body: some View { + if let message { + Text(message) + .font(.caption) + .foregroundStyle(isError ? Color.red : Color.secondary) + .multilineTextAlignment(.center) + .fixedSize(horizontal: false, vertical: true) + .padding(.horizontal, 12) + .padding(.top, 2) + .frame(maxWidth: .infinity) + .accessibilityIdentifier("overlay-mapper-status") + .transition(.opacity) + } + } +} diff --git a/Sources/KeyPathAppKit/UI/Overlay/OverlayMapperSection.swift b/Sources/KeyPathAppKit/UI/Overlay/OverlayMapperSection.swift index d4465d92d..6b5c60f35 100644 --- a/Sources/KeyPathAppKit/UI/Overlay/OverlayMapperSection.swift +++ b/Sources/KeyPathAppKit/UI/Overlay/OverlayMapperSection.swift @@ -236,18 +236,7 @@ struct OverlayMapperSection: View { // The overlay saves on capture, so a refusal or a declined save // must be said here; otherwise the mapping just seems to vanish. - if let status = viewModel.statusMessage { - Text(status) - .font(.caption) - .foregroundStyle(viewModel.statusIsError ? Color.red : Color.secondary) - .multilineTextAlignment(.center) - .fixedSize(horizontal: false, vertical: true) - .padding(.horizontal, 12) - .padding(.top, 2) - .frame(maxWidth: .infinity) - .accessibilityIdentifier("overlay-mapper-status") - .transition(.opacity) - } + MapperStatusLine(message: viewModel.statusMessage, isError: viewModel.statusIsError) appMappingIndicators .saturation(Double(1 - fadeAmount)) diff --git a/Tests/KeyPathTests/Lint/OverlayMapperStatusLintTests.swift b/Tests/KeyPathTests/Lint/OverlayMapperStatusLintTests.swift new file mode 100644 index 000000000..b8be58c40 --- /dev/null +++ b/Tests/KeyPathTests/Lint/OverlayMapperStatusLintTests.swift @@ -0,0 +1,33 @@ +import Foundation +@preconcurrency import XCTest + +/// The overlay Mapper autosaves and has no Save button. A status the view model +/// sets but no overlay view reads is a declined save nobody can see — which is +/// how a media-key mapping "vanished" on 2026-10-20. Keep the wiring explicit. +final class OverlayMapperStatusLintTests: XCTestCase { + func testOverlayMapperRendersTheViewModelStatus() throws { + let section = try String(contentsOf: LintScanner.path("Sources/KeyPathAppKit/UI/Overlay/OverlayMapperSection.swift"), encoding: .utf8) + XCTAssertTrue( + section.contains("MapperStatusLine(message: viewModel.statusMessage, isError: viewModel.statusIsError)"), + "OverlayMapperSection must render MapperStatusLine from viewModel.statusMessage; the overlay has no Save button, so an unrendered status is an invisible refusal." + ) + let line = try String(contentsOf: LintScanner.path("Sources/KeyPathAppKit/UI/Overlay/MapperStatusLine.swift"), encoding: .utf8) + XCTAssertTrue(line.contains("Text(message)") && line.contains("\"overlay-mapper-status\""), + "MapperStatusLine must show the message as text under the overlay-mapper-status identifier.") + } + + func testMapperSaveNeverReturnsSilently() throws { + // Every early `return` in save() must be preceded by a status and a log line. + let file = LintScanner.path("Sources/KeyPathAppKit/UI/Mapper/MapperViewModel+RuleManagement.swift") + let contents = try String(contentsOf: file, encoding: .utf8) + let saveBody = contents.components(separatedBy: "func save(kanataManager:")[1] + .components(separatedBy: "\n func ")[0] + var silent: [Int] = [] + let lines = saveBody.components(separatedBy: "\n") + for (index, text) in lines.enumerated() where text.trimmingCharacters(in: .whitespaces) == "return" { + let window = lines[max(0, index - 6) ..< index].joined(separator: "\n") + if !(window.contains("statusMessage =") && window.contains("AppLogger.shared.")) { silent.append(index) } + } + XCTAssertTrue(silent.isEmpty, "save() returns without a status and a log line at save-body lines \(silent)") + } +} diff --git a/Tests/KeyPathTests/UI/MapperStatusLineTests.swift b/Tests/KeyPathTests/UI/MapperStatusLineTests.swift new file mode 100644 index 000000000..49a6fdcb0 --- /dev/null +++ b/Tests/KeyPathTests/UI/MapperStatusLineTests.swift @@ -0,0 +1,51 @@ +import AppKit +@testable import KeyPathAppKit +import SwiftUI +import XCTest + +/// The overlay Mapper saves on capture with no Save button, so its status line +/// is the only place a declined save is visible. These render it for real. +@MainActor +final class MapperStatusLineTests: XCTestCase { + private func host(_ message: String?, isError: Bool) -> NSHostingView { + let hosted = NSHostingView(rootView: MapperStatusLine(message: message, isError: isError).frame(width: 300)) + hosted.frame = CGRect(x: 0, y: 0, width: 300, height: 120) + hosted.layoutSubtreeIfNeeded() + return hosted + } + + /// Distinct colours drawn by the hosted view: a blank render has one. + private func distinctColours(in view: NSView) throws -> Set { + let rep = try XCTUnwrap(view.bitmapImageRepForCachingDisplay(in: view.bounds)) + view.cacheDisplay(in: view.bounds, to: rep) + var seen = Set() + for y in stride(from: 0, to: rep.pixelsHigh, by: 2) { + for x in stride(from: 0, to: rep.pixelsWide, by: 2) { + if let colour = rep.colorAt(x: x, y: y) { + let r = UInt32(colour.redComponent * 255), g = UInt32(colour.greenComponent * 255), b = UInt32(colour.blueComponent * 255) + seen.insert(r << 16 | g << 8 | b) + } + } + } + return seen + } + + func testAMessageTakesSpaceAndDrawsText() throws { + let hosted = host("Louder sends Volume Up, which the current backend can't send.", isError: true) + XCTAssertGreaterThan(hosted.fittingSize.height, 0, "a status takes space") + XCTAssertGreaterThan(try distinctColours(in: hosted).count, 1, "a status draws glyphs, not a blank") + } + + func testAnErrorIsDrawnInRed() throws { + let error = try distinctColours(in: host("Refused", isError: true)) + let plain = try distinctColours(in: host("Refused", isError: false)) + let reddish = error.filter { ($0 >> 16) & 0xFF > 150 && ($0 >> 8) & 0xFF < 110 && $0 & 0xFF < 110 } + XCTAssertFalse(reddish.isEmpty, "an error status draws in red") + XCTAssertNotEqual(error, plain) + } + + func testNoMessageRendersNothing() { + let hosted = host(nil, isError: false) + XCTAssertEqual(hosted.fittingSize.height, 0) + } +} diff --git a/Tests/KeyPathTests/UI/MediaKeyCaptureToSaveTests.swift b/Tests/KeyPathTests/UI/MediaKeyCaptureToSaveTests.swift new file mode 100644 index 000000000..756c05148 --- /dev/null +++ b/Tests/KeyPathTests/UI/MediaKeyCaptureToSaveTests.swift @@ -0,0 +1,72 @@ +import AppKit +@testable import KeyPathAppKit +@testable import KeyPathCore +import KeyPathRulesCore +@preconcurrency import XCTest + +/// A media key captured from a system-defined event becomes a rule the session +/// can post: the path from NSEvent through the Mapper to the generated config, +/// end to end, against the real bridge. +@MainActor +final class MediaKeyCaptureToSaveTests: KeyPathTestCase { + private var directory: URL! + private var coordinator: RuntimeCoordinator! + private var service: ConfigurationService! + + override func setUp() async throws { + try await super.setUp() + try SessionBridgeTestFixture.requireAvailable() + BackendCapabilityProfile.testOverride = SessionBridgeTestFixture.profile + directory = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + let collections = RuleCollectionStore.testStore(at: directory.appendingPathComponent("RuleCollections.json")) + let rules = CustomRulesStore.testStore(at: directory.appendingPathComponent("CustomRules.json")) + service = ConfigurationService.sessionTestService(configDirectory: directory.path, ruleCollectionStore: collections, customRulesStore: rules) + let manager = RuleCollectionsManager(ruleCollectionStore: collections, customRulesStore: rules, configurationService: service) + manager.ruleCollections = [] + manager.customRules = [] + try await service.saveRuleState(ruleCollections: [], customRules: [], collectionStore: collections, customStore: rules) + coordinator = RuntimeCoordinator(injectedConfigurationService: service, injectedRuleCollectionsManager: manager) + } + + override func tearDown() async throws { + BackendCapabilityProfile.testOverride = nil + if let directory { try? FileManager.default.removeItem(at: directory) } + coordinator = nil; service = nil; directory = nil + try await super.tearDown() + } + + /// `NSEvent.otherEvent(.systemDefined, subtype 8)` as the hardware sends it. + private func mediaEvent(nxKeyType: Int, down: Bool) throws -> NSEvent { + try XCTUnwrap(NSEvent.otherEvent( + with: .systemDefined, location: .zero, modifierFlags: [], timestamp: 0, windowNumber: 0, context: nil, + subtype: 8, data1: (nxKeyType << 16) | ((down ? 0xA : 0xB) << 8), data2: -1 + )) + } + + func testCaptureNamesTheMediaKeyAndIgnoresKeyUp() throws { + let mute = try XCTUnwrap(KeyboardCapture.mediaKeyPress(from: mediaEvent(nxKeyType: 7, down: true))) + XCTAssertEqual(mute.baseKey, "mute") + XCTAssertEqual(mute.keyCode, 1007) + XCTAssertNil(try KeyboardCapture.mediaKeyPress(from: mediaEvent(nxKeyType: 7, down: false))) + XCTAssertEqual(try XCTUnwrap(KeyboardCapture.mediaKeyPress(from: mediaEvent(nxKeyType: 0, down: true))).baseKey, "volumeup") + } + + func testCapturedMediaKeyBecomesAPostableRule() async throws { + for (nxKeyType, kanata) in [(7, "mute"), (0, "volu"), (2, "brup"), (16, "pp")] { + let press = try XCTUnwrap(KeyboardCapture.mediaKeyPress(from: mediaEvent(nxKeyType: nxKeyType, down: true))) + let vm = MapperViewModel() + vm.configure(kanataManager: coordinator) + vm.inputSequence = KeySequence(keys: [KeyPress(baseKey: "grave", modifiers: [], keyCode: 50)], captureMode: .single) + vm.outputSequence = KeySequence(keys: [press], captureMode: .single) + await vm.save(kanataManager: coordinator) + XCTAssertFalse(vm.statusIsError, "\(kanata): \(vm.statusMessage ?? "")") + XCTAssertEqual(vm.statusMessage?.hasPrefix("✓ Saved"), true, vm.statusMessage ?? "nil") + let rule = try XCTUnwrap(coordinator.ruleCollectionsManager.customRules.first { $0.input == "grv" || $0.input == "grave" }) + XCTAssertEqual(rule.action, .keystroke(key: kanata)) + let config = try String(contentsOfFile: service.configurationPath, encoding: .utf8) + XCTAssertTrue(config.contains(kanata), "\(kanata) missing from the generated config") + XCTAssertFalse(config.contains("(macro"), "a media key must never be typed as letters") + } + } +} diff --git a/docs/architecture/rules-admission-review.md b/docs/architecture/rules-admission-review.md index a6aa40db1..5659570f1 100644 --- a/docs/architecture/rules-admission-review.md +++ b/docs/architecture/rules-admission-review.md @@ -502,6 +502,15 @@ carries names and details rather than ids, and lives in KeyPathCore where the ve types cannot be seen. It is produced from verdicts in exactly one place (`AdmissionEngine.mappingConflicts`). +**Overlay presentation is covered too (2026-10-20).** The hardware check found that the +overlay Mapper — which saves on capture and has no Save button — never rendered the view +model's status, so a declined save looked like a mapping that vanished. The engine's +tests could not see that: they assert on the model. Now `MapperStatusLine` is rendered +for real (`MapperStatusLineTests`), a lint keeps the overlay reading it and `save` from +returning without a status and a log line (`OverlayMapperStatusLintTests`), and +`MediaKeyCaptureToSaveTests` drives a synthesized system-defined event through +`KeyboardCapture` → Mapper → save → generated config against the real bridge. + The engine and `PackDependencyChecker` are plain enums with no actor: both only read value types and the immutable registry. `driverlessUnavailableReason` lost the main-actor annotation that had followed from the engine's.