Housekeeping: retention, CI test gate, hygiene, docs reconciliation

.github/workflows/docker.yml

@@ -12,7 +12,31 @@ env:
   IMAGE_NAME: ${{ github.repository }}
 
 jobs:
+  test:
+    runs-on: ubuntu-latest
+    permissions:
+      contents: read
+
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+
+      - name: Set up Go
+        uses: actions/setup-go@v5
+        with:
+          go-version-file: go.mod
+
+      - name: go vet
+        run: go vet ./...
+
+      - name: go build
+        run: go build ./...
+
+      - name: go test
+        run: go test ./...
+
   build:
+    needs: test
     runs-on: ubuntu-latest
     permissions:
       contents: read
@@ -22,9 +46,6 @@ jobs:
       - name: Checkout
         uses: actions/checkout@v4
 
-      - name: Set up QEMU
-        uses: docker/setup-qemu-action@v3
-
       - name: Set up Docker Buildx
         uses: docker/setup-buildx-action@v3
 
@@ -48,7 +69,7 @@ jobs:
             type=sha,prefix=
 
       - name: Build and push
-        uses: docker/build-push-action@v5
+        uses: docker/build-push-action@v6
         with:
           context: .
           platforms: linux/amd64

.gitignore

@@ -9,6 +9,10 @@
 .vscode/
 *.swp
 *.swo
+*.sketch
+
+# dex task state
+.dex/
 
 # OS
 .DS_Store

Dockerfile

@@ -1,5 +1,5 @@
 # Build stage
-FROM golang:1.25-bookworm AS builder
+FROM golang:1.25.12-bookworm AS builder
 
 RUN apt-get update && apt-get install -y gcc libsqlite3-dev && rm -rf /var/lib/apt/lists/*
 

README.md

@@ -62,6 +62,50 @@ Shows:
 - Temperature history graph with threshold lines
 - Active workload hints
 
+## Safety
+
+While running, this controller puts the iDRAC into **manual fan mode** via raw
+IPMI OEM commands and is the **sole thing standing between your hardware and
+the BMC's fans idling**. It is designed so that as many failure modes as
+possible fail toward "the BMC keeps cooling the box," not toward "the fans
+stay wherever they were and the machine cooks":
+
+- **Abnormal exit restores auto mode.** SIGINT/SIGTERM, a fatal API server
+  error, and even a panic in the control loop all go through the same
+  shutdown path, which calls `RestoreAutoMode` before the process exits. A
+  config that exists but fails safety validation makes the process refuse to
+  start at all (see [Upgrading](#upgrading)) rather than run with unsafe
+  defaults — since manual mode is never enabled in that case, the BMC simply
+  keeps its own automatic control.
+- **Sensor loss fails up, not down.** A failed temperature read is never
+  treated as 0°C (which would ramp fans down). The controller holds the last
+  fan speed and, after `sensor_failure_limit` consecutive failures, hands
+  cooling back to BMC auto mode. This is recoverable: once sensor reads
+  succeed again, manual control is reclaimed automatically.
+- **Fan-write failure is a sticky fail-safe.** After `write_failure_limit`
+  consecutive failures to *write* a fan speed, control is handed back to BMC
+  auto mode and stays there until the process restarts — the controller does
+  not probe the write channel again mid-run, since that would flap the BMC
+  between manual and auto.
+- **Restore is retried until confirmed.** If the `RestoreAutoMode` call itself
+  fails (e.g. the BMC is unreachable), the controller keeps retrying on every
+  control-loop tick until it succeeds; `restore_pending` in `/api/status`
+  reflects this. Nothing re-enables manual mode while a restore is
+  unconfirmed.
+- **Critical temperatures bypass everything.** If `critical_cpu_temp` or
+  `critical_gpu_temp` is reached, fans jump straight to `max_speed`,
+  bypassing step ramping and any active manual override.
+
+**What this does *not* protect against:** a `SIGKILL` (`kill -9`) or a power
+loss bypasses the shutdown path entirely and cannot trigger a restore — the
+BMC's own thermal protections are the last line of defense in that case, same
+as on any other server. This controller is a cooling *policy* on top of the
+BMC, not a replacement for its built-in safety logic.
+
+Also note that the raw IPMI commands used here (`ipmitool raw 0x30 0x30 ...`)
+are **Dell iDRAC-specific OEM commands** — they are not portable to other
+vendors' BMCs.
+
 ## API Security
 
 The API binds `0.0.0.0` by default (required for container/bridge networking), so
@@ -124,10 +168,24 @@ Returns current system state including thresholds:
   "zone": "idle",
   "cpu_threshold": 65,
   "gpu_threshold": 60,
-  "idle_speed": 20
+  "idle_speed": 20,
+  "failsafe_active": false,
+  "failsafe_reason": "none",
+  "restore_pending": false,
+  "last_write_failed": false
 }
 ```
 
+The fail-safe fields (see [Safety](#safety) above):
+
+- `failsafe_active` — `true` once cooling has been handed back to BMC auto
+  mode (sensor loss or repeated fan-write failures).
+- `failsafe_reason` — `"none"`, `"sensor-loss"`, or `"write-failure"`.
+- `restore_pending` — `true` if fail-safe is active but the hand-back to BMC
+  auto mode has not yet been confirmed (the BMC may still be in manual mode);
+  the controller keeps retrying until this clears.
+- `last_write_failed` — `true` if the most recent fan-speed write failed.
+
 ### POST /api/hint
 
 Register a workload hint for proactive cooling:
@@ -180,6 +238,11 @@ Get temperature/fan history for graphing.
 
 See [config.example.yaml](config.example.yaml) for all options.
 
+**Logging**: the controller logs to stderr only — there is no `logging.level`
+or `logging.file` config. Under Docker, use `docker logs` and your Docker log
+driver's rotation settings (e.g. `json-file` with `max-size`/`max-file`) to
+capture and rotate logs.
+
 ### Key Settings
 
 ```yaml
@@ -189,6 +252,17 @@ fan_control:
   gpu_threshold: 60          # Increase fans when GPU exceeds this (°C)
   step_size: 10              # Fan increase per 5°C over threshold (%)
   cooldown_delay: 60         # Seconds below threshold before ramping down
+  # Emergency ramp trigger — required, must exceed the (effective) threshold
+  # above it or the service refuses to start. See Safety and Upgrading above.
+  critical_cpu_temp: 85
+  critical_gpu_temp: 90
+  # Fail-safe: consecutive failures before handing cooling back to BMC auto.
+  sensor_failure_limit: 3
+  write_failure_limit: 3
+
+storage:
+  path: "/var/lib/only-fan-controller/history.db"
+  retention_days: 30         # History readings older than this are pruned daily
 ```
 
 ### Example: Quiet Home Server
@@ -261,6 +335,37 @@ All config options can be overridden via environment variables:
 2. Configure via the Unraid Docker UI
 3. Or use Community Applications (search "Only Fan Controller")
 
+## Upgrading
+
+**smart-fan-controller → only-fan-controller rename.** The project (and the
+Docker image) were renamed from `smart-fan-controller` to
+`only-fan-controller`. If you have an existing deployment:
+
+- The container name changed from `smart-fan-controller` to
+  `only-fan-controller` — update any `docker stop`/`docker logs`/etc. scripts
+  that reference the old name.
+- The GHCR image path changed accordingly (`ghcr.io/<owner>/only-fan-controller`
+  instead of `.../smart-fan-controller`); update your `docker-compose.yml` /
+  `docker run` image reference.
+- `scripts/hint-client.sh` now reads `FAN_URL` instead of `SMART_FAN_URL` for
+  the controller base URL, but still honors `SMART_FAN_URL` as a fallback if
+  set — no forced script changes.
+
+**Config validation now refuses to start on unsafe configs.** If your config
+sets `cpu_threshold` (or `gpu_threshold`) at or above the corresponding
+critical temperature's default — most notably `cpu_threshold >= 85` without
+also setting `critical_cpu_temp` explicitly above it — the service now exits
+at startup with a clear error instead of silently falling back to default
+thresholds. Set `critical_cpu_temp`/`critical_gpu_temp` explicitly, above your
+configured thresholds, to fix this.
+
+**API token auth is a config-only addition.** Existing deployments keep
+working unchanged; see [API Security](#api-security) above.
+
+**History retention now defaults to 30 days.** The `readings` table is
+pruned automatically (see `storage.retention_days` in
+[config.example.yaml](config.example.yaml)); previously it grew unbounded.
+
 ## Requirements
 
 - Dell PowerEdge server with iDRAC (tested on R730)

SPEC.md

@@ -1,5 +1,34 @@
 # iDRAC Smart Fan Controller
 
+> **This is a historical design document, not current documentation.** It
+> describes the original plan and has been superseded by the actual code and
+> [README.md](README.md), which describe what was actually built and is kept
+> up to date. Where this document conflicts with the code, the code wins.
+> See "What was actually built" below for the most significant drift; the
+> rest of this document (decision-engine pseudocode, phase checklists, file
+> layout, etc.) is left as originally written and should be read as design
+> history, not a spec of current behavior.
+
+## What was actually built (vs. this document)
+
+- **API routes are `/api/...`, and there is no `PUT /config`.** e.g.
+  `GET /api/status`, `POST /api/hint`, `POST /api/override`, not the bare
+  `/status`, `/hint`, etc. shown below. Configuration is read-only via
+  `GET /api/config`; there is no endpoint to update it at runtime.
+- **Zone-based fan curves were never the control mechanism.** Fan control is
+  simple threshold + hysteresis (`cpu_threshold`/`gpu_threshold`,
+  `cooldown_delay`), not the zone lookup described in "Temperature Zones &
+  Fan Curves" below. `zones` still exist in config, but only for dashboard
+  display. The emergency ramp added later (`critical_cpu_temp`/
+  `critical_gpu_temp`, see [README.md](README.md#safety)) is a separate,
+  explicitly-validated trigger — it is not part of the zone system either.
+- **Trend calculation is telemetry only.** `cpu_trend`/`gpu_trend` are
+  reported in `/api/status` for visibility, but they do not feed back into
+  the fan-speed decision the way the "Decision Engine Logic" pseudocode below
+  implies.
+- **The dashboard is embedded static HTML served on the same port as the
+  API**, not a separate Vue/Svelte SPA on port 8087. See `internal/api/static/`.
+
 ## Overview
 
 An intelligent fan controller for Dell PowerEdge servers that goes beyond simple threshold-based control. Monitors CPU and GPU temperatures, accepts hints from external applications about upcoming workloads, and makes predictive decisions about fan speeds to maintain optimal cooling while minimizing noise.

cmd/controller/main.go

@@ -11,6 +11,7 @@ import (
 	"strings"
 	"sync"
 	"syscall"
+	"time"
 
 	"github.com/sethpjohnson/only-fan-controller/internal/api"
 	"github.com/sethpjohnson/only-fan-controller/internal/config"
@@ -128,6 +129,39 @@ func restoreOnce(restore func() error) func() error {
 	}
 }
 
+// cleanupShutdownTimeout bounds how long shutdown waits for the history
+// cleanup goroutine to stop. Thermal safety (restore()) always runs before
+// this wait starts, so a hung Cleanup query must not stall process exit.
+const cleanupShutdownTimeout = 5 * time.Second
+
+// runHistoryCleanup prunes readings older than retention on a fixed daily
+// interval until stopCh is closed. The initial cleanup (so a fresh start
+// doesn't wait a full day before its first prune) is run by the caller before
+// this goroutine is started; this loop only handles the recurring runs.
+func runHistoryCleanup(store *storage.Store, retention time.Duration, retentionDays int, stopCh <-chan struct{}, done chan<- struct{}) {
+	defer close(done)
+	ticker := time.NewTicker(24 * time.Hour)
+	defer ticker.Stop()
+	for {
+		select {
+		case <-ticker.C:
+			logCleanupResult(store, retention, retentionDays)
+		case <-stopCh:
+			return
+		}
+	}
+}
+
+// logCleanupResult runs a single history cleanup pass and logs the outcome.
+func logCleanupResult(store *storage.Store, retention time.Duration, retentionDays int) {
+	deleted, err := store.Cleanup(retention)
+	if err != nil {
+		log.Printf("History cleanup failed: %v", err)
+		return
+	}
+	log.Printf("History cleanup: removed %d old reading(s) (retention %d days)", deleted, retentionDays)
+}
+
 // resolveConfig loads the config, distinguishing a genuinely absent file (safe
 // to fall back to defaults + env overrides) from a present-but-invalid file
 // (parse or safety-validation failure). For the latter we REFUSE to start rather
@@ -189,6 +223,17 @@ func run() int {
 	}
 	defer store.Close()
 
+	// Prune history once at startup, then keep pruning daily for as long as the
+	// process runs. This is not on the safety-critical restore path, so its
+	// goroutine is stopped (via cleanupStop/cleanupDone) before the deferred
+	// store.Close() above runs, but is otherwise independent of the
+	// control-loop/API shutdown below.
+	retention := time.Duration(cfg.Storage.RetentionDays) * 24 * time.Hour
+	logCleanupResult(store, retention, cfg.Storage.RetentionDays)
+	cleanupStop := make(chan struct{})
+	cleanupDone := make(chan struct{})
+	go runHistoryCleanup(store, retention, cfg.Storage.RetentionDays, cleanupStop, cleanupDone)
+
 	// errCh carries any fatal error (API server failure, control-loop panic)
 	// back to the shutdown path so cleanup + auto-mode restore always run.
 	errCh := make(chan error, 1)
@@ -249,6 +294,20 @@ func run() int {
 		log.Printf("Warning: Failed to restore auto fan mode: %v", err)
 	}
 
+	// Stop the history cleanup goroutine before the deferred store.Close() runs.
+	// Bounded: if a Cleanup query happens to be in flight, don't let it stall
+	// exit indefinitely (thermal safety already happened above, in restore()).
+	close(cleanupStop)
+	select {
+	case <-cleanupDone:
+	case <-time.After(cleanupShutdownTimeout):
+		// Abandoning it is safe: an in-flight Cleanup Exec racing the deferred
+		// store.Close() is benign — database/sql won't force-interrupt a
+		// checked-out connection, an interrupted DELETE rolls back via SQLite's
+		// journal, and the process exits immediately after this anyway.
+		log.Printf("Warning: history cleanup goroutine did not stop within %s; abandoning it", cleanupShutdownTimeout)
+	}
+
 	log.Println("Shutdown complete")
 	return exitCode
 }

config.example.yaml

@@ -25,6 +25,18 @@ fan_control:
   gpu_threshold: 60          # Bump fans when GPU exceeds this (°C)
   step_size: 10              # Fan speed increment per 5°C over threshold (%)
   cooldown_delay: 60         # Seconds below threshold before ramping down
+  # Critical temperatures force an emergency ramp straight to max_speed,
+  # bypassing step ramping and any manual override. Required, and must be
+  # greater than the (effective) threshold above it — the service refuses to
+  # start otherwise rather than silently falling back to defaults. If you
+  # raise cpu_threshold/gpu_threshold, raise these too.
+  critical_cpu_temp: 85      # Emergency ramp at/above this CPU temp (°C)
+  critical_gpu_temp: 90      # Emergency ramp at/above this GPU temp (°C)
+  # Fail-safe thresholds: after this many consecutive failures, cooling is
+  # handed back to the BMC's automatic fan control until reads/writes succeed
+  # again. See the README's Safety section for the full fail-safe design.
+  sensor_failure_limit: 3    # Consecutive sensor read failures before restoring auto mode
+  write_failure_limit: 3     # Consecutive fan-write failures before restoring auto mode
 
 api:
   # host 0.0.0.0 binds every interface — REQUIRED for container/bridge
@@ -51,7 +63,10 @@ dashboard:
 
 storage:
   path: "/var/lib/only-fan-controller/history.db"
+  # How long history readings are kept before being pruned. Cleanup runs once
+  # at startup and then once a day. Must be > 0.
+  retention_days: 30
 
-logging:
-  level: "info"              # debug, info, warn, error
-  file: "/var/log/only-fan-controller.log"
+# Logs go to stderr; there is no file-logging config. Docker/systemd/your
+# process supervisor is responsible for capturing and rotating them (e.g.
+# `docker logs`, or the json-file/local log driver's rotation options).

docker-compose.yml

@@ -1,5 +1,3 @@
-version: '3.8'
-
 services:
   only-fan-controller:
     build: .