From 06a4461e95089229d5bad0bd1c45a8892c824978 Mon Sep 17 00:00:00 2001 From: iBobby Date: Tue, 20 Oct 2026 12:37:16 -0600 Subject: [PATCH] core: confirm the hands-off handback against hardware mode with a bounded watch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Switching to a hands-off profile submitted resetAuto and cleared the monitor's fansCurrentlyRunning flag on bookkeeping alone. A fan write already in flight (a ramp tick racing the switch across the monitor/main queues) can land at the daemon AFTER that resetAuto and re-pin the fans in manual — and nothing ever corrects it: the flag-driven hands-off branch sees "not running" and stays silent, and with the app alive and heartbeating the daemon watchdog never reclaims the orphaned hold. The fans then stay pinned in manual until quit. Switching to hands-off (or Reset to Default) now arms a 10-second handback watch instead of submitting a bare resetAuto: hands-off ticks read the hardware fan mode, re-send resetAuto at 1Hz while any fan still reads manual, and settle once auto is confirmed. The watch keeps running past a confirmed-auto reading so a late re-pin is still caught inside the window, and it is bounded — a dead daemon or an external tool legitimately holding manual is never fought forever. Status reading is injected (defaulting to the fanControl reader) and the monitor queue / tick are internal, so the handback is unit-tested hardware-free: re-pin before confirm, re-pin after confirm, clean handback, idle hands-off quiet, and watch expiry. --- Sources/ThermalForgeApp/AppState.swift | 12 +- Sources/ThermalForgeCore/ThermalMonitor.swift | 68 +++++-- .../ThermalMonitorHandbackTests.swift | 185 ++++++++++++++++++ 3 files changed, 253 insertions(+), 12 deletions(-) create mode 100644 Tests/ThermalForgeTests/ThermalMonitorHandbackTests.swift diff --git a/Sources/ThermalForgeApp/AppState.swift b/Sources/ThermalForgeApp/AppState.swift index cd15b5c4..a4b3d7d6 100644 --- a/Sources/ThermalForgeApp/AppState.swift +++ b/Sources/ThermalForgeApp/AppState.swift @@ -420,6 +420,10 @@ final class AppState: ObservableObject { // here, on the daemon-confirmed success path, never on a failed reset. self.persistSelectedProfile(FanProfile.silent.id) self.monitor?.switchProfile(.silent) + // The daemon confirmed THIS resetAuto, but a fan write already in + // flight can still land after it and re-pin manual — arm the + // bounded watch so the handback is confirmed against hardware. + self.monitor?.armHandbackWatch() TFLogger.shared.profile("Reset to Default (Silent (Apple Default))") } } @@ -436,7 +440,13 @@ final class AppState: ObservableObject { // a CLI hold (so its unsupervised hold isn't orphaned). Otherwise active // profiles let tick() ramp from the current temperature. Off-main one-shot // on the pump (never coalesced/reordered). - if profile.curve.handsOff || profile.id == "smart" || profile.id == "silent" || took { + if profile.curve.handsOff { + // No direct resetAuto: a fan write already in flight can land at the + // daemon after it and re-pin manual. The bounded handback watch + // confirms auto against the hardware and retries as needed — it also + // covers a CLI-hold takeover, whose release is the same resetAuto. + monitor?.armHandbackWatch() + } else if profile.id == "smart" || took { commandPump.submit(.resetAuto) } } diff --git a/Sources/ThermalForgeCore/ThermalMonitor.swift b/Sources/ThermalForgeCore/ThermalMonitor.swift index 39b0d947..0dd823e3 100644 --- a/Sources/ThermalForgeCore/ThermalMonitor.swift +++ b/Sources/ThermalForgeCore/ThermalMonitor.swift @@ -48,9 +48,15 @@ public enum MonitorState: Equatable { // MARK: - Thermal Monitor public final class ThermalMonitor { - private let fanControl: FanControl private var timer: DispatchSourceTimer? - private let queue = DispatchQueue(label: "com.thermalforge.monitor") + + /// The monitor's work queue. Internal (not private) so unit tests can run + /// tick() ON the queue and use a sync hop as a barrier behind async work. + let queue = DispatchQueue(label: "com.thermalforge.monitor") + + /// Hardware status source — injected so unit tests can script fan modes + /// without SMC; production always passes the fanControl-backed reader. + private let readStatus: () throws -> ThermalStatus public private(set) var activeProfile: FanProfile public private(set) var state: MonitorState = .idle @@ -75,6 +81,15 @@ public final class ThermalMonitor { private var lastAppliedRPMPercent: Float = 0 private var fansCurrentlyRunning = false + + /// Hands-off handback retry cadence: while the hardware still reads manual + /// on every 100ms tick, retry resetAuto at 1Hz — a healthy handback + /// confirms on the next tick, and a 10Hz retry would only hammer the + /// daemon. Internal for unit tests. + static let handsOffRetryCadence = 10 + + /// Ticks remaining in an armed handback watch (see armHandbackWatch). + private var handbackTicksLeft = 0 private var sustainedAboveCount = 0 // MARK: - Smart Profile State @@ -112,10 +127,15 @@ public final class ThermalMonitor { /// Called when a fan command needs to be executed (may require privilege) public var onFanCommand: ((FanCommand) throws -> Void)? - public init(fanControl: FanControl, profile: FanProfile = .silent) { - self.fanControl = fanControl + public convenience init(fanControl: FanControl, profile: FanProfile = .silent) { + self.init(profile: profile, readStatus: { try fanControl.status() }) + } + + init(profile: FanProfile = .silent, + readStatus: @escaping () throws -> ThermalStatus) { self.activeProfile = profile self.tickInterval = 0.1 + self.readStatus = readStatus } // MARK: - Lifecycle @@ -137,6 +157,17 @@ public final class ThermalMonitor { timer = nil } + /// Arm a bounded handback watch. For the next `seconds`, hands-off ticks + /// CONFIRM the handback to Apple auto against the hardware fan mode + /// instead of assuming it from our own bookkeeping, retrying resetAuto + /// while any fan still reads manual. Bounded, so a dead daemon or an + /// external tool legitimately holding manual is never fought forever. + public func armHandbackWatch(seconds: Float = 10) { + queue.async { [self] in + handbackTicksLeft = Int(seconds / tickInterval) + } + } + /// Update the active profile. public func switchProfile(_ profile: FanProfile) { queue.async { [self] in @@ -164,8 +195,10 @@ public final class ThermalMonitor { // MARK: - Polling - private func tick() { - guard let status = try? fanControl.status() else { return } + /// One thermal tick. Internal (not private) so unit tests can run it ON + /// the monitor queue, where it is designed to run. + func tick() { + guard let status = try? readStatus() else { return } latestStatus = status // Peak CPU (TC/Tp) + GPU (TG/Tg) — the shared safety-floor sensor extraction, @@ -416,12 +449,25 @@ public final class ThermalMonitor { let maxRPM = status.fans.first.map { Float($0.maxRPM) } ?? 7826 let minRPM = status.fans.first.map { Float($0.minRPM) } ?? 2317 - // Hands-off profiles (Silent): don't control fans, just monitor + // Hands-off profiles (Silent): don't control fans, just monitor. The + // handback to Apple auto is CONFIRMED against the hardware fan mode, + // not assumed from our own bookkeeping: a fan write already in flight + // when the profile switched can land at the daemon AFTER the switch's + // resetAuto and re-pin the fans in manual — switchProfile already + // cleared fansCurrentlyRunning, so a flag-driven branch would never + // correct the orphaned hold, and with the app alive and heartbeating + // the daemon watchdog never reclaims it either: the fans stay pinned + // in manual until quit. Supervision is bounded: the switch arms a + // handback watch, and during the window a manual reading re-sends + // resetAuto (rate-limited) — also catching a re-pin that lands after + // the hardware briefly confirmed auto. if curve.handsOff { - if fansCurrentlyRunning { - applyCommand(.resetAuto) - fansCurrentlyRunning = false - lastAppliedRPMPercent = 0 + let watching = handbackTicksLeft > 0 + if watching { handbackTicksLeft -= 1 } + if watching, status.fans.contains(where: { $0.mode == "manual" }) { + if tickCounter % Self.handsOffRetryCadence == 0 { + applyCommand(.resetAuto) + } state = .idle } return diff --git a/Tests/ThermalForgeTests/ThermalMonitorHandbackTests.swift b/Tests/ThermalForgeTests/ThermalMonitorHandbackTests.swift new file mode 100644 index 00000000..7141fec8 --- /dev/null +++ b/Tests/ThermalForgeTests/ThermalMonitorHandbackTests.swift @@ -0,0 +1,185 @@ +// +// ThermalMonitorHandbackTests.swift +// ThermalForge +// +// Hands-off handback is CONFIRMED against the hardware fan mode, not assumed +// from bookkeeping: a ramp write already in flight when the profile switched +// can land at the daemon AFTER the switch's resetAuto and re-pin the fans in +// manual — both BEFORE the first confirmation tick and after the hardware has +// briefly read auto (the 2026-10-10 race took the second shape). The armed +// handback watch must retry resetAuto until the hardware confirms auto, then +// settle; and it must EXPIRE, so a hands-off monitor never permanently fights +// a dead daemon or an external tool holding manual. +// + +import Foundation +import Testing + +@testable import ThermalForgeCore + +@Suite("ThermalMonitor hands-off handback") +struct ThermalMonitorHandbackTests { + + /// Thread-safe command recorder — ticks run on the monitor's queue while + /// assertions read from the test thread. + final class Recorder: @unchecked Sendable { + private let lock = NSLock() + private var _commands: [FanCommand] = [] + + var commands: [FanCommand] { + lock.lock(); defer { lock.unlock() } + return _commands + } + + func record(_ c: FanCommand) { + lock.lock(); defer { lock.unlock() } + _commands.append(c) + } + + var resetAutoCount: Int { + commands.filter { $0 == .resetAuto }.count + } + } + + /// Scripted status source: one fan whose mode the test flips between + /// manual (a hold is pinning it) and auto (handed back to Apple). + final class StatusScript: @unchecked Sendable { + private let lock = NSLock() + private var _mode: String + private let temp: Float + + init(mode: String = "manual", temp: Float = 85) { + _mode = mode + self.temp = temp + } + + var mode: String { + get { lock.lock(); defer { lock.unlock() }; return _mode } + set { lock.lock(); defer { lock.unlock() }; _mode = newValue } + } + + var status: ThermalStatus { + ThermalStatus( + fans: [.init(index: 0, actualRPM: 4024, targetRPM: 4024, + minRPM: 2317, maxRPM: 7826, mode: mode)], + temperatures: ["TC0p": temp] + ) + } + } + + private func makeMonitor(profile: FanProfile, script: StatusScript, + rec: Recorder) -> ThermalMonitor { + let m = ThermalMonitor(profile: profile, readStatus: { script.status }) + m.onFanCommand = { rec.record($0) } + return m + } + + /// Run one tick ON the monitor's queue (where tick() is designed to run), + /// which also acts as a barrier behind any queued switch/watch arm. + private func tick(_ m: ThermalMonitor) { + m.queue.sync { m.tick() } + } + + /// Balanced profile driven past its 8-second sustained trigger, fans + /// running: the pre-switch state of the race (a ramp is active, the + /// monitor believes it owns the fans). + private func rampThenSwitchToSilent(_ m: ThermalMonitor, rec: Recorder) { + for _ in 0..<85 { tick(m) } // 80 ticks = balanced's sustained window + #expect(rec.commands.contains { if case .setRPM = $0 { true } else { false } }) + m.switchProfile(.silent) + m.armHandbackWatch() // what selectProfile does on a switch + m.queue.sync {} // barrier: let both land + } + + @Test("re-pin BEFORE the first hands-off tick: watch retries resetAuto until the hardware confirms auto") + func repinBeforeConfirmIsHealed() { + let script = StatusScript() // manual — the late write already landed + let rec = Recorder() + let m = makeMonitor(profile: .balanced, script: script, rec: rec) + + rampThenSwitchToSilent(m, rec: rec) + + // Retries are rate-limited to one per handsOffRetryCadence ticks; drive + // enough ticks to cross at least one retry boundary. + let before = rec.resetAutoCount + for _ in 0..<(ThermalMonitor.handsOffRetryCadence * 2 + 2) { tick(m) } + let retried = rec.resetAutoCount - before + #expect(retried >= 1) + #expect(retried <= 3) // rate-limited, not hammered + + // The retry landed: hardware reads auto — the loop settles and then + // never sends anything again. + script.mode = "auto" + for _ in 0..<3 { tick(m) } + let settled = rec.commands.count + for _ in 0..<5 { tick(m) } + #expect(rec.commands.count == settled) + } + + @Test("re-pin AFTER the hardware confirmed auto: the still-armed watch catches it") + func repinAfterConfirmIsHealed() { + let script = StatusScript() + let rec = Recorder() + let m = makeMonitor(profile: .balanced, script: script, rec: rec) + + rampThenSwitchToSilent(m, rec: rec) + script.mode = "auto" // the switch's resetAuto landed fine + tick(m) // hardware-confirmed settle, no send + let settled = rec.commands.count + #expect(rec.resetAutoCount == 0) + + // The late ramp write lands NOW — after the confirmed auto. The watch + // is still armed and must correct the re-pin. + script.mode = "manual" + for _ in 0..<(ThermalMonitor.handsOffRetryCadence + 2) { tick(m) } + #expect(rec.resetAutoCount >= 1) + + script.mode = "auto" // correction landed — settle again + for _ in 0..<3 { tick(m) } + let settledAgain = rec.commands.count + for _ in 0..<5 { tick(m) } + #expect(rec.commands.count == settledAgain) + } + + @Test("clean handback: hardware already auto — no retry, state settles, nothing sent") + func cleanHandbackSettlesImmediately() { + let script = StatusScript() + let rec = Recorder() + let m = makeMonitor(profile: .balanced, script: script, rec: rec) + + rampThenSwitchToSilent(m, rec: rec) + script.mode = "auto" + + for _ in 0..<6 { tick(m) } // confirms and settles on the first + #expect(rec.resetAutoCount == 0) // no redundant resetAuto + } + + @Test("idle hands-off (never controlled) never sends anything — even in manual") + func idleSilentStaysQuiet() { + // E.g. an external tool holds the fans manual; a Silent ThermalForge + // that never controlled them (and never armed a watch) must not fight it. + let script = StatusScript(mode: "manual") + let rec = Recorder() + let m = makeMonitor(profile: .silent, script: script, rec: rec) + + for _ in 0..<15 { tick(m) } + #expect(rec.commands.isEmpty) + } + + @Test("the handback watch is bounded: retries stop when it expires") + func watchExpiresAndStopsRetrying() { + let script = StatusScript() // manual forever — daemon dead + let rec = Recorder() + let m = makeMonitor(profile: .balanced, script: script, rec: rec) + + rampThenSwitchToSilent(m, rec: rec) + + // Default window is 10s = 100 ticks at 100ms; at 1 Hz that is ~10 + // retries. Run PAST the window, then prove nothing more is sent. + for _ in 0..<110 { tick(m) } + let expired = rec.resetAutoCount + #expect(expired >= 8 && expired <= 12) // retried through the window… + for _ in 0..<30 { tick(m) } + #expect(rec.resetAutoCount == expired) // …and then stopped + } +}