cli: route fan commands through the privileged daemon (stacked on #29) - #32
Open
titan550 wants to merge 2 commits into
Open
cli: route fan commands through the privileged daemon (stacked on #29)#32titan550 wants to merge 2 commits into
titan550 wants to merge 2 commits into
Conversation
added 2 commits
July 17, 2026 23:26
`auto` ran `killall ThermalForgeApp` unconditionally so the fan reset would stick against a running profile. But that meant any programmatic caller — e.g. a server restoring fans to auto on shutdown — silently killed the user's menu bar app. The daemon's own `auto` path never did this. Gate the app-quit behind a new `--stop-app` flag (default off), so `auto` is a pure fan-reset command. Interactive users who want the old behavior can pass `--stop-app`.
`max`, `set` (all fans), and `auto` now go through the daemon socket when the daemon is running, so they work without sudo and coordinate with the menu bar app instead of racing it for SMC control. Direct SMC writes remain the fallback when no daemon is installed. One-shot CLI holds send a trailing `nohb` token so the daemon leaves its heartbeat watchdog disarmed; without it the 15s watchdog would silently revert a fire-and-forget `thermalforge max` to auto. The menu bar app stays supervised (crash-protected) as before. Single-fan `set --fan N` still needs direct SMC access: per-fan control is not exposed over the daemon socket protocol. Stacked on fix/auto-app-kill-optin.
titan550
marked this pull request as ready for review
July 19, 2026 01:06
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The CLI's
max,set, andautoalways write to the SMC directly, which requires root. On a machine where the ThermalForge daemon is already running as root — the normal installed state — an unprivilegedthermalforge maxfails with "Fan unlock failed: Timed out setting fan 0 to manual mode. Run with sudo," because AppleSMC accepts reads but the firmware rejects every fan-mode write, and the unlock loop times out. Callers are forced into sudo (or fail) while a privileged path that already speaksmax/set/autosits idle on/tmp/thermalforge.sock. Direct writes also race the app→daemon control loop instead of coordinating with it.Change
FanCommandRouter.apply: one entry point that uses the daemon when it's running (no sudo, coordinates with the app) and falls back to direct SMC writes otherwise (daemon-less installs behave exactly as before, including the sudo requirement).max,set(all fans), andautogo through the router. Single-fanset --fan Nintentionally stays direct-to-SMC — per-fan control is not part of the socket protocol — and still needs sudo.nohb(unsupervised hold): one-shot CLI commands append anohbtoken so the daemon leaves its heartbeat watchdog disarmed; without it, the 15s watchdog silently reverts a fire-and-forgetthermalforge maxto auto. The menu bar app omits the token and stays supervised/crash-protected exactly as today.Compatibility
Older daemons tokenize on whitespace and ignore the trailing
nohbformax/set, so a new CLI against an old daemon degrades to today's behavior (hold reverts after the watchdog window) rather than erroring. There is no CLI↔daemon version handshake in the protocol — that's noted here as future work rather than smuggled into this PR.Build-verified with
swift build -c release; the daemon-routedmax/status/autosequence was exercised end-to-end against a live root daemon on an M5 Max (fans commanded to 5349/5777 RPM without sudo, then restored).