fix(ui): stop serving a cold snapshot cache as an empty sites list

internal/ui/server.go

@@ -750,7 +750,7 @@ func buildStatus() StatusResponse {
 	}
 }
 
-func buildStatusJSON() []byte { return []byte(mustJSON(buildStatus())) }
+func buildStatusJSON() ([]byte, error) { return []byte(mustJSON(buildStatus())), nil }
 
 // WorktreeResponse is embedded in SiteResponse for each git worktree.
 // PHP/NodeVersion are the effective values; *Override flags signal whether
@@ -916,16 +916,30 @@ type SiteResponse struct {
 }
 
 func handleSites(w http.ResponseWriter, _ *http.Request) {
+	// A nil snapshot means the registry could not be read and there is nothing
+	// cached to fall back on. Saying so beats writing a zero-byte body the
+	// dashboard would render as "you have no sites".
+	body := snapshots.Sites()
+	if body == nil {
+		http.Error(w, "sites are temporarily unavailable, the registry could not be read", http.StatusServiceUnavailable)
+		return
+	}
 	w.Header().Set("Content-Type", "application/json")
-	_, _ = w.Write(snapshots.Sites())
+	_, _ = w.Write(body)
 }
 
-func buildSitesJSON() []byte { return []byte(mustJSON(buildSites())) }
+func buildSitesJSON() ([]byte, error) {
+	sites, err := buildSites()
+	if err != nil {
+		return nil, err
+	}
+	return []byte(mustJSON(sites)), nil
+}
 
-func buildSites() []SiteResponse {
+func buildSites() ([]SiteResponse, error) {
 	enriched, err := siteinfo.LoadAll(siteinfo.EnrichUI)
 	if err != nil {
-		return []SiteResponse{}
+		return nil, fmt.Errorf("loading sites: %w", err)
 	}
 	_ = siteinfo.PersistVersionChanges(enriched)
 
@@ -1140,7 +1154,7 @@ func buildSites() []SiteResponse {
 			Workspace:       resolveSiteWorkspace(e, groupMainName, siteWorkspace),
 		})
 	}
-	return sites
+	return sites, nil
 }
 
 // ServicePortMapping describes one published port of a service: its
@@ -1524,7 +1538,7 @@ func handleServices(w http.ResponseWriter, _ *http.Request) {
 	_, _ = w.Write(snapshots.Services())
 }
 
-func buildServicesJSON() []byte { return []byte(mustJSON(buildServicesList())) }
+func buildServicesJSON() ([]byte, error) { return []byte(mustJSON(buildServicesList())), nil }
 
 func buildServicesList() []ServiceResponse {
 	// One ss/lsof call shared across all installed-but-stopped services in

internal/ui/snapshot.go

@@ -1,6 +1,7 @@
 package ui
 
 import (
+	"log"
 	"sync"
 	"time"
 
@@ -29,188 +30,128 @@ func currentSnapshotTTL() time.Duration {
 	return snapshotTTLIdle
 }
 
-// snapshotCache holds cached JSON bytes of the last /api/sites, /api/services,
-// and /api/status responses. Handlers read from here instead of rebuilding
-// from scratch on every poll; /api/ws broadcasts the same bytes to every
-// connected browser.
-type snapshotCache struct {
-	mu sync.Mutex
-
-	sites, services, status       []byte
-	sitesAt, servicesAt, statusAt time.Time
-
-	// Unhealthy-workers JSON shadows the sites cycle: it is derived from
-	// the same batched unit-state cache and invalidated alongside KindSites,
-	// so callers don't pay any extra subprocess cost.
-	unhealthy   []byte
-	unhealthyAt time.Time
-
-	// One build-mutex per kind serialises concurrent rebuilds so that when
-	// podman inspect is slow, goroutines queue behind one in-flight rebuild
-	// rather than each spawning their own batch of subprocesses.
-	sitesBuild, servicesBuild, statusBuild, unhealthyBuild sync.Mutex
+// snapshotSlot caches one kind's JSON bytes behind its own build mutex, so
+// that when podman inspect is slow the callers queue behind a single in-flight
+// rebuild rather than each spawning their own batch of subprocesses.
+//
+// get returns nil only when there is nothing truthful to say: no cached value
+// and a build that failed. Callers must treat nil as "unavailable" rather than
+// as an empty result, because presenting it as data is how a cold cache turned
+// into a dashboard with no sites on it.
+type snapshotSlot struct {
+	mu    sync.Mutex
+	build sync.Mutex
+
+	data []byte
+	at   time.Time
+
+	fn func() ([]byte, error)
 }
 
-var snapshots = &snapshotCache{}
-
-// Sites returns cached /api/sites JSON, rebuilding if stale.
-// If a rebuild is already in progress, returns the stale value immediately
-// rather than queuing behind the in-flight build.
-func (c *snapshotCache) Sites() []byte {
-	c.mu.Lock()
-	if c.sites != nil && time.Since(c.sitesAt) < currentSnapshotTTL() {
-		b := c.sites
-		c.mu.Unlock()
+func (s *snapshotSlot) get() []byte {
+	s.mu.Lock()
+	if s.data != nil && time.Since(s.at) < currentSnapshotTTL() {
+		b := s.data
+		s.mu.Unlock()
 		return b
 	}
-	stale := c.sites
-	c.mu.Unlock()
-
-	if !c.sitesBuild.TryLock() {
-		return stale
-	}
-	defer c.sitesBuild.Unlock()
-
-	c.mu.Lock()
-	if c.sites != nil && time.Since(c.sitesAt) < currentSnapshotTTL() {
-		b := c.sites
-		c.mu.Unlock()
-		return b
+	stale := s.data
+	s.mu.Unlock()
+
+	// A caller holding a usable value never waits on a slow rebuild. A caller
+	// with nothing has to, or it answers with the cold cache's nil and the
+	// dashboard renders that as an empty list.
+	if stale != nil {
+		if !s.build.TryLock() {
+			return stale
+		}
+	} else {
+		s.build.Lock()
 	}
-	c.mu.Unlock()
-
-	b := buildSitesJSON()
-	c.mu.Lock()
-	c.sites = b
-	c.sitesAt = time.Now()
-	c.mu.Unlock()
-	return b
-}
+	defer s.build.Unlock()
 
-// Services returns cached /api/services JSON, rebuilding if stale.
-// If a rebuild is already in progress, returns the stale value immediately
-// rather than queuing behind the in-flight build.
-func (c *snapshotCache) Services() []byte {
-	c.mu.Lock()
-	if c.services != nil && time.Since(c.servicesAt) < currentSnapshotTTL() {
-		b := c.services
-		c.mu.Unlock()
+	s.mu.Lock()
+	if s.data != nil && time.Since(s.at) < currentSnapshotTTL() {
+		b := s.data
+		s.mu.Unlock()
 		return b
 	}
-	stale := c.services
-	c.mu.Unlock()
+	s.mu.Unlock()
 
-	if !c.servicesBuild.TryLock() {
+	b, err := s.fn()
+	if err != nil {
+		// Storing this would serve one transient failure as the truth for a
+		// whole TTL, up to five minutes when no tab is counted visible.
+		log.Printf("[snapshot] rebuild failed, keeping previous value: %v", err)
 		return stale
 	}
-	defer c.servicesBuild.Unlock()
 
-	c.mu.Lock()
-	if c.services != nil && time.Since(c.servicesAt) < currentSnapshotTTL() {
-		b := c.services
-		c.mu.Unlock()
-		return b
-	}
-	c.mu.Unlock()
-
-	b := buildServicesJSON()
-	c.mu.Lock()
-	c.services = b
-	c.servicesAt = time.Now()
-	c.mu.Unlock()
+	s.mu.Lock()
+	s.data = b
+	s.at = time.Now()
+	s.mu.Unlock()
 	return b
 }
 
-// Status returns cached /api/status JSON, rebuilding if stale.
-// If a rebuild is already in progress, returns the stale value immediately
-// rather than queuing behind the in-flight build.
-func (c *snapshotCache) Status() []byte {
-	c.mu.Lock()
-	if c.status != nil && time.Since(c.statusAt) < currentSnapshotTTL() {
-		b := c.status
-		c.mu.Unlock()
-		return b
-	}
-	stale := c.status
-	c.mu.Unlock()
+// invalidate drops the cached value's freshness so the next read rebuilds.
+func (s *snapshotSlot) invalidate() {
+	s.mu.Lock()
+	s.at = time.Time{}
+	s.mu.Unlock()
+}
 
-	if !c.statusBuild.TryLock() {
-		return stale
-	}
-	defer c.statusBuild.Unlock()
+// snapshotCache holds the cached JSON of the last /api/sites, /api/services,
+// and /api/status responses. Handlers read from here instead of rebuilding
+// from scratch on every poll; /api/ws broadcasts the same bytes to every
+// connected browser.
+type snapshotCache struct {
+	sites, services, status snapshotSlot
 
-	c.mu.Lock()
-	if c.status != nil && time.Since(c.statusAt) < currentSnapshotTTL() {
-		b := c.status
-		c.mu.Unlock()
-		return b
-	}
-	c.mu.Unlock()
+	// Worker health shadows the sites cycle: it is derived from the same
+	// batched unit-state cache and invalidated alongside KindSites, so callers
+	// don't pay any extra subprocess cost.
+	unhealthy snapshotSlot
+}
 
-	b := buildStatusJSON()
-	c.mu.Lock()
-	c.status = b
-	c.statusAt = time.Now()
-	c.mu.Unlock()
-	return b
+var snapshots = &snapshotCache{
+	sites:     snapshotSlot{fn: buildSitesJSON},
+	services:  snapshotSlot{fn: buildServicesJSON},
+	status:    snapshotSlot{fn: buildStatusJSON},
+	unhealthy: snapshotSlot{fn: buildUnhealthyWorkersJSON},
 }
 
-// UnhealthyWorkers returns cached worker-health JSON, rebuilding if stale.
-// Pinned to the KindSites lifecycle: same source cache, same invalidation
-// signal, no extra polling.
-func (c *snapshotCache) UnhealthyWorkers() []byte {
-	c.mu.Lock()
-	if c.unhealthy != nil && time.Since(c.unhealthyAt) < currentSnapshotTTL() {
-		b := c.unhealthy
-		c.mu.Unlock()
-		return b
-	}
-	stale := c.unhealthy
-	c.mu.Unlock()
+// Sites returns cached /api/sites JSON, rebuilding if stale. nil means no
+// snapshot could be produced.
+func (c *snapshotCache) Sites() []byte { return c.sites.get() }
 
-	if !c.unhealthyBuild.TryLock() {
-		return stale
-	}
-	defer c.unhealthyBuild.Unlock()
+// Services returns cached /api/services JSON, rebuilding if stale.
+func (c *snapshotCache) Services() []byte { return c.services.get() }
 
-	c.mu.Lock()
-	if c.unhealthy != nil && time.Since(c.unhealthyAt) < currentSnapshotTTL() {
-		b := c.unhealthy
-		c.mu.Unlock()
-		return b
-	}
-	c.mu.Unlock()
+// Status returns cached /api/status JSON, rebuilding if stale.
+func (c *snapshotCache) Status() []byte { return c.status.get() }
 
-	b := buildUnhealthyWorkersJSON()
-	c.mu.Lock()
-	c.unhealthy = b
-	c.unhealthyAt = time.Now()
-	c.mu.Unlock()
-	return b
-}
+// UnhealthyWorkers returns cached worker-health JSON, rebuilding if stale.
+// Pinned to the KindSites lifecycle: same source cache, same invalidation
+// signal, no extra polling.
+func (c *snapshotCache) UnhealthyWorkers() []byte { return c.unhealthy.get() }
 
 // Invalidate drops the cached bytes for one kind so the next read rebuilds.
 func (c *snapshotCache) Invalidate(kind string) {
-	c.mu.Lock()
-	defer c.mu.Unlock()
 	switch kind {
 	case eventbus.KindSites:
-		c.sitesAt = time.Time{}
-		// Worker health shares the sites lifecycle.
-		c.unhealthyAt = time.Time{}
+		c.sites.invalidate()
+		c.unhealthy.invalidate()
 	case eventbus.KindServices:
-		c.servicesAt = time.Time{}
+		c.services.invalidate()
 	case eventbus.KindStatus:
-		c.statusAt = time.Time{}
+		c.status.invalidate()
 	}
 }
 
-// InvalidateAll drops all three cached snapshots.
+// InvalidateAll drops all cached snapshots.
 func (c *snapshotCache) InvalidateAll() {
-	c.mu.Lock()
-	c.sitesAt = time.Time{}
-	c.servicesAt = time.Time{}
-	c.statusAt = time.Time{}
-	c.unhealthyAt = time.Time{}
-	c.mu.Unlock()
+	c.sites.invalidate()
+	c.services.invalidate()
+	c.status.invalidate()
+	c.unhealthy.invalidate()
 }