Fix the tooltips, the speedtest engine's bookkeeping, and the recovery paths

The settings tooltips said several things the daemon does not do - a wrong
retry floor, a power button that supposedly stopped manual actions, colour
and logging switches described backwards - and the overgrown ones buried
what decides a toggle. All corrected and cut to size: the Best of 3 tooltip
was a forty-line essay in a 250-pixel bubble, most of it a scoring formula
the README already documents. Warnings were added where decisions get made
(retries at 0 also disable the slow-uplink rescue; the packet-loss probe's
traffic is not counted in the data-used figure), and unbacked advice was
replaced with measured numbers.

The server picker judged health with a cheap read check a server can pass
while refusing every actual upload; our fleet survey found about one server
in seven refuses them, and when such a server is nearest it won every
selection and the speed history sat empty forever. A finished run's own
evidence now feeds back: a run where the server answered every upload and
accepted none marks it unusable for 12 hours. What does not count is
deliberate - a starved slow uplink, a redirect, overload answers (429, 502,
503, 504), any accepted upload, and any run we cancelled ourselves. The
verdict holds its ground: a health probe already in flight when the run
convicted the server cannot land late and quietly overwrite it - expiry is
the only readmission - and when several DIFFERENT servers get convicted in
quick succession, the log says what that pattern means: a firewall or proxy
on the operator's own network is probably filtering upload POSTs. Uploads
themselves are recognized by what they are - the engine's only POSTs - so
no server's choice of endpoint path can blind that evidence chain. And a
server answering the endpoint probe with a relative redirect (the ordinary
nginx shape) is followed to its working endpoint instead of blacklisted for
a perfectly legal header.

A speedtest that measured its download and lost only its upload used to be
thrown away whole - and on uplinks under roughly 3 Mbps the upload always
fails, so those users had a permanently empty chart. Such a run is now
stored as a partial, like the iperf3 engine has always done: download, ping
and jitter land on the chart, the upload shows as unmeasured rather than a
fake zero, and the failed upload's traffic is still billed through the
accounting channel that cannot masquerade as a measurement. The boundaries
hold everywhere: an upload-only test still fails outright, a cancelled run
is recorded as cancelled, the packet-loss probe is skipped (it would measure
through the failed upload's leftovers), and in a best-of round a partial
winner's blank upload stays blank instead of inheriting other servers'
bytes. And a direction the run tried and LOST now counts as a breach of any
minimum configured on it, in both engines - the alternative judged such
runs healthy, silencing the upload alert with the very failure it watches
and wiping an in-progress alert streak at the moment the uplink fell off
the cliff.

A speedtest row's timestamp is its identity everywhere - deleting a run,
merging a backup, linking chart to table - but the accounting rows could
land on an occupied second, where deleting one row silently un-billed
another and a restore quietly dropped one of the two. Every insert now
takes the first free second, in one atomic statement protected by the same
busy-wait as every ordinary write: the first version used a read-then-write
transaction, the exact shape this database already broke on once, and under
the daemon's own probe traffic it dropped a COMPLETED measurement within a
handful of inserts on a real file-backed database (the in-memory test
databases run a single connection and could never see it; a file-backed
test now hammers exactly that). The spend row is also written only after
the measurement it references, so a crash can no longer leave a permanent
orphan billing bytes for a run that never landed.

The outage repair could corrupt a neighbouring outage's record. It paired
outages only two minutes into the future while its own design notes work to
a 48-hour horizon, and its correction grabbed the first recovery row after
an outage began without checking whether a different outage started in
between - given a complete future pair from a fast clock, it rewrote that
pair's recovery as the old outage's and left a permanently open outage that
never happened. The pairing now reaches the full 48 hours, corrections are
made by row identity so nothing can be seized, and a future-dated recovery
whose proving samples are pruned in the same pass is moved back to the
proven second - while one whose samples survive stays exactly where the
operator's history put it.

The documented lockout recovery for containers - restart with
PINGULARITY_ACCESS=network - only worked when the boot's first settings
read succeeded. The settings controller now runs the access sequence after
every successful load through a single hook, so the retry loop after a
transient database fault, the reload signal, and a settings import or
backup restore all apply the operator's explicit choice; the reload signal
no longer bails on the harmless legacy-password warning, one shared rule
defines a usable load for every path, and the first-load flag is read under
the writer lock so two racing first loads cannot both claim to be first.
Nothing changes for installs without the explicit flag.

And whether Quick Setup has been answered is decided from the loaded
settings before any database read: a passing disk fault at boot used to put
a long-since-answered install back on the first-run hold, pausing
monitoring and announcing a first run to an operator who answered it months
ago. Answered installs also stop paying a database read on every probe
round's hold check.

Co-Authored-By: Claude <noreply@anthropic.com>

Author: cansofgrease

README.md

@@ -677,8 +677,10 @@ switching engines switches which pair is in force. On Ookla, retries are also wh
 let a very slow uplink finish at all: when parallel upload streams are too slow for
 any of them to complete inside the capture window, the retry falls back to a single
 stream. Set Ookla's retries to `0` and that fallback cannot run, so on a link that
-slow the upload - and with it the whole run - fails. The error says so and names the
-setting.
+slow the upload always fails and records nothing. The run's download half is kept
+either way - a "both" run that loses only its upload stores its download, ping and
+jitter as a partial result, with the upload shown as unmeasured (the same contract
+iperf3 has always had) - and the warning in the log says why and names the setting.
 
 That UDP pass needs the iperf3 port open for **UDP as well as TCP** - the same
 port, both protocols (`ufw allow 5201/tcp` and `ufw allow 5201/udp`, or the

internal/settings/quicksetup_test.go

@@ -4,6 +4,8 @@ import (
 	"context"
 	"errors"
 	"strconv"
+	"sync"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -499,3 +501,41 @@ func TestQuickSetupOfferSinceErr(t *testing.T) {
 		t.Error("a store read error must be surfaced, not masked as 0 (the hold must fail-safe)")
 	}
 }
+
+// TestConcurrentFirstLoadsFireOneFirstLoadHook: wasLoaded is read under wmu
+// inside reload, so of N concurrent Reloads racing an unloaded controller,
+// exactly ONE observes the unloaded->loaded transition. Sampled outside the
+// lock, several could - each firing the boot-shaped ambiguity warning and a
+// store-wide read the firstLoad gate exists to avoid.
+func TestConcurrentFirstLoadsFireOneFirstLoadHook(t *testing.T) {
+	ctx := context.Background()
+	st, err0 := store.Open(":memory:")
+	if err0 != nil {
+		t.Fatal(err0)
+	}
+	t.Cleanup(func() { st.Close() })
+	dead, cancel := context.WithCancel(ctx)
+	cancel()
+	c, err := New(dead, st, Values{Latency: 5 * time.Second, Speed: time.Hour, Timeout: 2 * time.Second, DownAfter: 3, UpAfter: 2})
+	if err == nil {
+		t.Fatal("fixture: initial load must fail")
+	}
+	var firstLoads atomic.Int64
+	c.OnLoaded(func(firstLoad bool) {
+		if firstLoad {
+			firstLoads.Add(1)
+		}
+	})
+	var wg sync.WaitGroup
+	for i := 0; i < 8; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			_ = c.Reload(ctx)
+		}()
+	}
+	wg.Wait()
+	if got := firstLoads.Load(); got != 1 {
+		t.Fatalf("%d hooks fired with firstLoad=true across 8 concurrent reloads, want exactly 1", got)
+	}
+}

internal/settings/settings.go

@@ -385,6 +385,15 @@ type Controller struct {
 	// persisting the blank, so an unrelated form save can't erase a still-recoverable
 	// password. Rebuilt each load; set under wmu in New/Reload, read under wmu in mutate.
 	sealedByAddr map[string]string
+	// onLoaded, when set, is called after every load that leaves the controller
+	// usable (Reload returning nil or ErrLegacyReseal), OUTSIDE the controller's
+	// locks, with firstLoad reporting whether this load took the controller from
+	// unloaded to loaded. It exists so the process can hang behavior off "settings
+	// (re)loaded" without every call site of Reload - the boot retry loop, the
+	// reload signal, the web import - having to remember to invoke it: a forgotten
+	// site is how an explicit access override once silently stopped applying.
+	// Set once, before any concurrent Reload can fire.
+	onLoaded atomic.Pointer[func(firstLoad bool)]
 	// initErr records that the initial settings read failed, so the controller is
 	// running on defaults rather than stored config. Writes are refused (see
 	// mutate) to avoid clobbering the stored config with defaults, and the web
@@ -671,6 +680,17 @@ func New(ctx context.Context, st *store.Store, def Values, opts ...Option) (*Con
 	return c, nil
 }
 
+// LoadedOK is the ONE definition of "this load left the controller usable":
+// a clean load, or ErrLegacyReseal - settings fully live, only the legacy
+// iperf3 password re-encryption failed. The boot path, the retry loop, the
+// reload signal, the web import and the controller's own post-load hook all
+// judge loads with it; scattered inline copies once disagreed, and the reload
+// signal silently skipped the whole post-load sequence on exactly the
+// installs where nothing else would run it.
+func LoadedOK(err error) bool {
+	return err == nil || errors.Is(err, ErrLegacyReseal)
+}
+
 // ErrLegacyReseal marks a settings load that SUCCEEDED but could not rewrite
 // legacy plaintext iperf3 passwords in their sealed form. Settings are in
 // effect; only the at-rest re-encryption is pending.
@@ -897,11 +917,33 @@ func overlay(v Values, m map[string]string) Values {
 // writer lock so a concurrent setter can't slip between the DB read and the
 // broadcast and have its change stomped by stale data.
 func (c *Controller) Reload(ctx context.Context) error {
+	// wasLoaded comes from reload itself, read under wmu: sampled out here,
+	// two concurrent first loads (the boot retry loop racing a SIGHUP or a
+	// web import) both saw unloaded and both fired the hook with
+	// firstLoad=true - a duplicate boot-shaped warning and a duplicate
+	// store-wide read, the exact cost gating on firstLoad exists to avoid.
+	wasLoaded, err := c.reload(ctx)
+	if LoadedOK(err) {
+		// Fire the post-load hook with every lock released: the hook may write
+		// settings (the access override does), and a write takes wmu.
+		if fn := c.onLoaded.Load(); fn != nil {
+			(*fn)(!wasLoaded)
+		}
+	}
+	return err
+}
+
+// OnLoaded registers the post-load hook (see the field). Call before any
+// concurrent Reload is possible.
+func (c *Controller) OnLoaded(fn func(firstLoad bool)) { c.onLoaded.Store(&fn) }
+
+func (c *Controller) reload(ctx context.Context) (wasLoaded bool, err error) {
 	c.wmu.Lock()
 	defer c.wmu.Unlock()
+	wasLoaded = c.Loaded() // under wmu: serialized against every other load
 	m, err := c.store.AllSettings(ctx)
 	if err != nil {
-		return err
+		return wasLoaded, err
 	}
 	// Reload is the recovery path's first successful read when New's failed (the
 	// settings-retry loop, SIGHUP): the fresh install it just recovered starts
@@ -929,15 +971,15 @@ func (c *Controller) Reload(ctx context.Context) error {
 			// Sealing failed: don't rewrite the imported passwords in the clear. The
 			// config is already live in memory (broadcast above); report the same
 			// distinguishable ErrLegacyReseal a failed write would.
-			return fmt.Errorf("%w: %v", ErrLegacyReseal, err)
+			return wasLoaded, fmt.Errorf("%w: %v", ErrLegacyReseal, err)
 		}
 		if _, err := c.store.SetSettingsDiff(ctx, map[string]string{
 			keyIperfServers: sealed,
 		}); err != nil {
-			return fmt.Errorf("%w: %v", ErrLegacyReseal, err)
+			return wasLoaded, fmt.Errorf("%w: %v", ErrLegacyReseal, err)
 		}
 	}
-	return nil
+	return wasLoaded, nil
 }
 
 // Getters (each safe for concurrent use).

internal/speedtest/iperf.go

@@ -691,6 +691,11 @@ func (i *Iperf) Run(ctx context.Context) (Result, error) {
 		// rate instead of being invisible.
 		if dir == "both" && (dnErr != nil || upErr != nil) {
 			stats.Inc("speed.iperf_partial")
+			// Recorded on the Result so a configured threshold on the FAILED
+			// direction reads as a breach instead of being silenced by the
+			// very failure it watches (see Result.UploadFailed).
+			res.DownloadFailed = dnErr != nil
+			res.UploadFailed = upErr != nil
 			if i.Log != nil {
 				if dnErr != nil {
 					i.Log.Warn("iperf3 direction failed, partial result kept", "direction", "down", "err", i.withEnvHint(dnErr))