From 3c6b434d780c0671e92e481f9e6d3f1451efefd1 Mon Sep 17 00:00:00 2001 From: andyhtran <76441965+andyhtran@users.noreply.github.com> Date: Sat, 8 Aug 2026 12:35:00 -0400 Subject: [PATCH] Require two consecutive over-threshold readings before rebuilding a starved tap A single latency sample can be stale after a long quiet stretch (observed on macOS 26), so the watchdog now treats the first bad reading as suspect and only convicts on a second consecutive one. Reading the tap list resets the window server's accumulator, which is what makes the confirmation reliable: a stale sample cannot survive its own read. --- .../Services/Hotkeys/ModifierTapMonitor.swift | 38 +++++++-- .../Services/Hotkeys/TapHealthPolicy.swift | 42 ++++++++-- .../TapHealthPolicyTests.swift | 79 ++++++++++++++----- 3 files changed, 125 insertions(+), 34 deletions(-) diff --git a/Sources/MiniWhisper/Services/Hotkeys/ModifierTapMonitor.swift b/Sources/MiniWhisper/Services/Hotkeys/ModifierTapMonitor.swift index 41f69c9..5069094 100644 --- a/Sources/MiniWhisper/Services/Hotkeys/ModifierTapMonitor.swift +++ b/Sources/MiniWhisper/Services/Hotkeys/ModifierTapMonitor.swift @@ -37,6 +37,10 @@ final class ModifierTapMonitor: @unchecked Sendable { /// Written from the tap thread, read by the watchdog; both under `lock`. private var lastEventTime: CFAbsoluteTime = CFAbsoluteTimeGetCurrent() private var starvedRebuilds = 0 + /// True after one over-threshold latency read; the next tick either + /// confirms starvation or proves the sample was stale. Main-actor confined + /// like `starvedRebuilds` — only `checkTapHealth` touches either. + private var starvationSuspected = false private var watchdogTimer: Timer? private var retryTimer: Timer? @@ -195,12 +199,14 @@ final class ModifierTapMonitor: @unchecked Sendable { // resolve. Without a retry here nothing would rebuild the tap short // of restarting the app. log.info("Modifier tap missing while it should be running; retrying creation") + starvationSuspected = false createTap() return } if !CGEvent.tapIsEnabled(tap: tap) { log.warning("Watchdog found modifier tap disabled; re-enabling") + starvationSuspected = false CGEvent.tapEnable(tap: tap, enable: true) if !CGEvent.tapIsEnabled(tap: tap) { log.error("Re-enable did not stick; recreating modifier tap") @@ -212,16 +218,28 @@ final class ModifierTapMonitor: @unchecked Sendable { // Reaching here means the tap claims to be enabled, which a starved tap // also does — hence the second, external opinion below. let latencyUs = reportedTapLatencyUs() - if TapStarvationPolicy.isStarved(silentFor: silent, reportedLatencyUs: latencyUs) { + let latencySeconds = Int((latencyUs ?? 0) / 1_000_000) + switch TapStarvationPolicy.verdict( + silentFor: silent, + reportedLatencyUs: latencyUs, + wasSuspect: starvationSuspected + ) { + case .healthy: + starvationSuspected = false + starvedRebuilds = 0 + case .suspect: + starvationSuspected = true + log.warning( + "Tap latency reads \(latencySeconds)s after \(Int(silent))s of silence; re-checking next tick before rebuilding — the sample may be stale" + ) + case .starved: + starvationSuspected = false starvedRebuilds += 1 - let latencySeconds = Int((latencyUs ?? 0) / 1_000_000) log.error( - "Tap starved — enabled but WindowServer queue latency \(latencySeconds)s; recreating (rebuild #\(self.starvedRebuilds) since last healthy tick)" + "Tap starved — enabled but WindowServer queue latency \(latencySeconds)s on two consecutive checks; recreating (rebuild #\(self.starvedRebuilds) since last healthy tick)" ) recreate() - return } - starvedRebuilds = 0 } /// The window server's queue latency for this tap, matched by tapping pid @@ -229,8 +247,14 @@ final class ModifierTapMonitor: @unchecked Sendable { /// and the Fn companion observer both use different masks), and only this /// one's health is being judged. /// - /// The value grows in lockstep with wall clock while an event sits - /// undelivered, which is what separates a starved tap from an idle one. + /// Two properties of this figure shape the policy above. While an event + /// sits undelivered it grows with wall clock, which is the signal that + /// separates a starved tap from an idle one. But after a long quiet + /// stretch it can also hold one enormous stale sample (observed on + /// macOS 26 while input was healthy), so a single reading is never acted + /// on. Reading the list resets the accumulator, which is what makes the + /// two-read confirmation work: a stale sample cannot survive its own + /// read, a genuine backlog is high again by the next tick. private func reportedTapLatencyUs() -> Float? { var count: UInt32 = 0 guard CGGetEventTapList(0, nil, &count) == .success, count > 0 else { return nil } diff --git a/Sources/MiniWhisper/Services/Hotkeys/TapHealthPolicy.swift b/Sources/MiniWhisper/Services/Hotkeys/TapHealthPolicy.swift index 5222002..3c638da 100644 --- a/Sources/MiniWhisper/Services/Hotkeys/TapHealthPolicy.swift +++ b/Sources/MiniWhisper/Services/Hotkeys/TapHealthPolicy.swift @@ -19,15 +19,43 @@ enum TapStarvationPolicy { /// rotting in the queue while the tap still claims to be enabled. static let starvedLatencyUs: Float = 5_000_000 - /// A rebuild needs both signals: prolonged silence *and* the window server - /// reporting queued-but-unserviced events. Either alone has a benign - /// explanation — an idle keyboard, or a latency sample taken across a - /// sleep/wake. + /// One over-threshold sample is an accusation, not a conviction. + enum Verdict: Equatable, Sendable { + /// No evidence of starvation; any prior suspicion is withdrawn. + case healthy + /// First over-threshold sample. The read that produced it reset the + /// window server's accumulator, so the next tick re-measures from a + /// clean slate: a stale sample cannot survive to the second read, a + /// genuine backlog can. + case suspect + /// Over threshold on two consecutive reads: rebuild. + case starved + } + + /// A rebuild needs three signals: prolonged silence, the window server + /// reporting queued-but-unserviced events, and that report surviving a + /// second look. + /// + /// The second look exists because the latency figure is least trustworthy + /// exactly when it is consulted. After a long quiet stretch the accumulator + /// can hold one enormous stale sample — many minutes of "latency" have been + /// observed on macOS 26 while input was demonstrably healthy — and the + /// silence gate means quiet stretches are the only time this code asks. + /// Reading the list resets the accumulator, so re-checking one tick later + /// separates the artifact from a real backlog, at the cost of one watchdog + /// interval of extra delay before a true rebuild. /// /// `reportedLatencyUs` is nil when the tap could not be found in the window /// server's list, which is not evidence of starvation. - static func isStarved(silentFor: CFTimeInterval, reportedLatencyUs: Float?) -> Bool { - guard silentFor > silenceThreshold, let latency = reportedLatencyUs else { return false } - return latency > starvedLatencyUs + static func verdict( + silentFor: CFTimeInterval, + reportedLatencyUs: Float?, + wasSuspect: Bool + ) -> Verdict { + guard silentFor > silenceThreshold, + let latency = reportedLatencyUs, + latency > starvedLatencyUs + else { return .healthy } + return wasSuspect ? .starved : .suspect } } diff --git a/Tests/MiniWhisperTests/TapHealthPolicyTests.swift b/Tests/MiniWhisperTests/TapHealthPolicyTests.swift index cc5f38a..16013fb 100644 --- a/Tests/MiniWhisperTests/TapHealthPolicyTests.swift +++ b/Tests/MiniWhisperTests/TapHealthPolicyTests.swift @@ -5,48 +5,87 @@ import Testing /// The rebuild decision for a tap that still reports itself as enabled. Getting /// this wrong in either direction is costly: a false positive tears down a -/// healthy tap every watchdog tick, a false negative leaves the shortcut +/// healthy tap on a lying latency sample, a false negative leaves the shortcut /// silently dead until the app restarts. +/// +/// The verdict is deliberately two-stage. The latency figure can hold one +/// enormous stale sample after a quiet stretch — exactly the moment the +/// silence gate lets it be consulted — so one bad reading only raises +/// suspicion, and only a second consecutive bad reading convicts. struct TapStarvationPolicyTests { - private let starved = TapStarvationPolicy.starvedLatencyUs + 1 + private let overThreshold = TapStarvationPolicy.starvedLatencyUs + 1 private let healthy: Float = 250 // µs, a normal serviced tap + private let longSilence = TapStarvationPolicy.silenceThreshold + 60 - @Test func idleKeyboardAloneIsNotStarvation() { + @Test func idleKeyboardAloneIsHealthy() { #expect( - !TapStarvationPolicy.isStarved( - silentFor: TapStarvationPolicy.silenceThreshold + 60, - reportedLatencyUs: healthy)) + TapStarvationPolicy.verdict( + silentFor: longSilence, + reportedLatencyUs: healthy, + wasSuspect: false) == .healthy) } - @Test func highLatencyOnARecentlyActiveTapIsNotStarvation() { - #expect(!TapStarvationPolicy.isStarved(silentFor: 1, reportedLatencyUs: starved)) + @Test func highLatencyOnARecentlyActiveTapIsHealthy() { + #expect( + TapStarvationPolicy.verdict( + silentFor: 1, + reportedLatencyUs: overThreshold, + wasSuspect: false) == .healthy) } - @Test func silenceAndQueuedEventsTogetherMeanStarvation() { + @Test func firstOverThresholdReadingOnlyRaisesSuspicion() { #expect( - TapStarvationPolicy.isStarved( - silentFor: TapStarvationPolicy.silenceThreshold + 1, - reportedLatencyUs: starved)) + TapStarvationPolicy.verdict( + silentFor: longSilence, + reportedLatencyUs: overThreshold, + wasSuspect: false) == .suspect) + } + + @Test func secondConsecutiveOverThresholdReadingConvicts() { + #expect( + TapStarvationPolicy.verdict( + silentFor: longSilence, + reportedLatencyUs: overThreshold, + wasSuspect: true) == .starved) + } + + /// The reading that raised suspicion reset the window server's accumulator, + /// so a healthy second reading proves the first sample was stale — the + /// suspicion must not linger and convict on some later, unrelated reading. + @Test func aHealthySecondReadingWithdrawsSuspicion() { + #expect( + TapStarvationPolicy.verdict( + silentFor: longSilence, + reportedLatencyUs: healthy, + wasSuspect: true) == .healthy) } /// No entry in the window server's list is missing evidence, not evidence - /// of starvation. + /// of starvation — with or without prior suspicion. @Test func anUnreportedTapIsNeverJudgedStarved() { #expect( - !TapStarvationPolicy.isStarved( - silentFor: TapStarvationPolicy.silenceThreshold + 1_000, - reportedLatencyUs: nil)) + TapStarvationPolicy.verdict( + silentFor: longSilence + 1_000, + reportedLatencyUs: nil, + wasSuspect: false) == .healthy) + #expect( + TapStarvationPolicy.verdict( + silentFor: longSilence + 1_000, + reportedLatencyUs: nil, + wasSuspect: true) == .healthy) } @Test func thresholdsAreExclusive() { #expect( - !TapStarvationPolicy.isStarved( + TapStarvationPolicy.verdict( silentFor: TapStarvationPolicy.silenceThreshold, - reportedLatencyUs: TapStarvationPolicy.starvedLatencyUs + 1)) + reportedLatencyUs: overThreshold, + wasSuspect: true) == .healthy) #expect( - !TapStarvationPolicy.isStarved( + TapStarvationPolicy.verdict( silentFor: TapStarvationPolicy.silenceThreshold + 1, - reportedLatencyUs: TapStarvationPolicy.starvedLatencyUs)) + reportedLatencyUs: TapStarvationPolicy.starvedLatencyUs, + wasSuspect: true) == .healthy) } /// Guards the two constants themselves: the latency bar has to sit above