device-health-oracle: address code review findings

- Close ClickHouse client on shutdown (defer chClient.Close())
- Validate database name against [a-zA-Z0-9_-]+ to prevent injection
- Use status.IsDrained() instead of == for forward compatibility
- Pass tick timestamp through BurnInTimes.Now instead of per-device
  time.Now() calls
- Add comment explaining 2-tick minimum for ReadyForUsers progression
- Remove obvious comments per CLAUDE.md guidelines
- Add test for expectedMinutes == 0 edge case (new environment)

Author: nikw9944

controlplane/device-health-oracle/cmd/device-health-oracle/main.go

@@ -147,6 +147,7 @@ func main() {
 		if err != nil {
 			log.Warn("ClickHouse connection failed, continuing without controller_success criterion", "addr", chAddr, "error", err)
 		} else {
+			defer chClient.Close()
 			log.Info("ClickHouse enabled", "addr", chAddr, "db", chDB, "user", chUser, "tls", !chTLSDisabled)
 			controllerSuccess := worker.NewControllerSuccessCriterion(chClient, log)
 			deviceCriteria = append(deviceCriteria, controllerSuccess)

controlplane/device-health-oracle/internal/worker/clickhouse.go

@@ -4,12 +4,15 @@ import (
 	"context"
 	"crypto/tls"
 	"fmt"
+	"regexp"
 	"strings"
 	"time"
 
 	"github.com/ClickHouse/clickhouse-go/v2"
 )
 
+var validDBName = regexp.MustCompile(`^[a-zA-Z0-9_-]+$`)
+
 // ControllerCallChecker queries ClickHouse for controller call records.
 type ControllerCallChecker interface {
 	ControllerCallCoverage(ctx context.Context, devicePubkey string, start, end time.Time) (minutesWithCalls int64, err error)
@@ -22,8 +25,11 @@ type ClickHouseClient struct {
 	db   string
 }
 
-// NewClickHouseClient creates a new ClickHouse client connection.
 func NewClickHouseClient(addr, db, user, pass string, disableTLS bool) (*ClickHouseClient, error) {
+	if !validDBName.MatchString(db) {
+		return nil, fmt.Errorf("invalid clickhouse database name: %q", db)
+	}
+
 	addr = strings.TrimPrefix(addr, "https://")
 	addr = strings.TrimPrefix(addr, "http://")
 
@@ -74,7 +80,6 @@ func (c *ClickHouseClient) ControllerCallCoverage(ctx context.Context, devicePub
 	return int64(minutesWithCalls), nil
 }
 
-// Close closes the ClickHouse connection.
 func (c *ClickHouseClient) Close() error {
 	return c.conn.Close()
 }

controlplane/device-health-oracle/internal/worker/controller_success.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"log/slog"
-	"time"
 
 	"github.com/gagliardetto/solana-go"
 	"github.com/malbeclabs/doublezero/smartcontract/sdk/go/serviceability"
@@ -21,7 +20,6 @@ type ControllerSuccessCriterion struct {
 	log     *slog.Logger
 }
 
-// NewControllerSuccessCriterion creates a new ControllerSuccessCriterion.
 func NewControllerSuccessCriterion(checker ControllerCallChecker, log *slog.Logger) *ControllerSuccessCriterion {
 	return &ControllerSuccessCriterion{
 		checker: checker,
@@ -34,7 +32,7 @@ func (c *ControllerSuccessCriterion) Name() string {
 }
 
 func (c *ControllerSuccessCriterion) Check(ctx context.Context, device serviceability.Device) (bool, string) {
-	start, expectedMinutes, ok := DeviceBurnIn(ctx, device.Status)
+	start, now, expectedMinutes, ok := DeviceBurnIn(ctx, device.Status)
 	if !ok {
 		return false, "burn-in times not available in context"
 	}
@@ -43,7 +41,7 @@ func (c *ControllerSuccessCriterion) Check(ctx context.Context, device serviceab
 	}
 
 	pubkey := solana.PublicKeyFromBytes(device.PubKey[:]).String()
-	minutesWithCalls, err := c.checker.ControllerCallCoverage(ctx, pubkey, start, time.Now())
+	minutesWithCalls, err := c.checker.ControllerCallCoverage(ctx, pubkey, start, now)
 	if err != nil {
 		c.log.Error("Failed to query controller call coverage",
 			"device", pubkey, "code", device.Code, "error", err)

controlplane/device-health-oracle/internal/worker/controller_success_test.go

@@ -37,6 +37,7 @@ func TestControllerSuccessCriterion_Passes(t *testing.T) {
 	start := now.Add(-33 * time.Minute)
 	ctx := ContextWithBurnInTimes(context.Background(), BurnInTimes{
 		DrainedStart: start,
+		Now:          now,
 	})
 
 	checker := &mockControllerCallChecker{minutesWithCalls: 33}
@@ -53,6 +54,7 @@ func TestControllerSuccessCriterion_Fails_InsufficientCoverage(t *testing.T) {
 	start := now.Add(-33 * time.Minute)
 	ctx := ContextWithBurnInTimes(context.Background(), BurnInTimes{
 		DrainedStart: start,
+		Now:          now,
 	})
 
 	checker := &mockControllerCallChecker{minutesWithCalls: 20}
@@ -69,6 +71,7 @@ func TestControllerSuccessCriterion_Fails_ClickHouseError(t *testing.T) {
 	start := now.Add(-1 * time.Hour)
 	ctx := ContextWithBurnInTimes(context.Background(), BurnInTimes{
 		ProvisioningStart: start,
+		Now:               now,
 	})
 
 	checker := &mockControllerCallChecker{err: errors.New("connection refused")}
@@ -95,6 +98,7 @@ func TestControllerSuccessCriterion_UsesProvisioningStart(t *testing.T) {
 	ctx := ContextWithBurnInTimes(context.Background(), BurnInTimes{
 		ProvisioningStart: now.Add(-60 * time.Minute),
 		DrainedStart:      now.Add(-10 * time.Minute),
+		Now:               now,
 	})
 
 	// Provide enough coverage for provisioning (60 min) but check that
@@ -112,6 +116,7 @@ func TestControllerSuccessCriterion_UsesDrainedStart(t *testing.T) {
 	ctx := ContextWithBurnInTimes(context.Background(), BurnInTimes{
 		ProvisioningStart: now.Add(-60 * time.Minute),
 		DrainedStart:      now.Add(-10 * time.Minute),
+		Now:               now,
 	})
 
 	// Only 10 minutes of coverage — enough for drained but not provisioning.
@@ -122,3 +127,22 @@ func TestControllerSuccessCriterion_UsesDrainedStart(t *testing.T) {
 	passed, _ := c.Check(ctx, device)
 	assert.True(t, passed)
 }
+
+func TestControllerSuccessCriterion_ZeroBurnIn_Passes(t *testing.T) {
+	// When burn-in start == now (zero-length window), the criterion should pass
+	// without querying ClickHouse. This happens in newly created environments
+	// where the burn-in slot is 0.
+	now := time.Now()
+	ctx := ContextWithBurnInTimes(context.Background(), BurnInTimes{
+		ProvisioningStart: now,
+		Now:               now,
+	})
+
+	checker := &mockControllerCallChecker{err: errors.New("should not be called")}
+	c := NewControllerSuccessCriterion(checker, testLoggerSlog())
+
+	device := serviceability.Device{Status: serviceability.DeviceStatusDeviceProvisioning}
+	passed, reason := c.Check(ctx, device)
+	assert.True(t, passed)
+	assert.Empty(t, reason)
+}

controlplane/device-health-oracle/internal/worker/criteria.go

@@ -15,6 +15,7 @@ type burnInTimesKey struct{}
 type BurnInTimes struct {
 	ProvisioningStart time.Time
 	DrainedStart      time.Time
+	Now               time.Time // tick timestamp, used as the end of the burn-in window
 }
 
 // ContextWithBurnInTimes returns a new context carrying the given BurnInTimes.
@@ -26,20 +27,17 @@ func ContextWithBurnInTimes(ctx context.Context, times BurnInTimes) context.Cont
 // start time and expected number of minutes for the given device status.
 // Returns ok=false if the context has no BurnInTimes, and expectedMinutes=0
 // when the burn-in window has zero length (e.g. a newly created environment).
-func DeviceBurnIn(ctx context.Context, status serviceability.DeviceStatus) (start time.Time, expectedMinutes int64, ok bool) {
+func DeviceBurnIn(ctx context.Context, status serviceability.DeviceStatus) (start time.Time, now time.Time, expectedMinutes int64, ok bool) {
 	burnIn, ok := ctx.Value(burnInTimesKey{}).(BurnInTimes)
 	if !ok {
-		return time.Time{}, 0, false
+		return time.Time{}, time.Time{}, 0, false
 	}
 	start = burnIn.ProvisioningStart
-	if status == serviceability.DeviceStatusDrained {
+	if status.IsDrained() {
 		start = burnIn.DrainedStart
 	}
-	expectedMinutes = int64(time.Since(start).Minutes())
-	if expectedMinutes < 0 {
-		expectedMinutes = 0
-	}
-	return start, expectedMinutes, true
+	expectedMinutes = max(int64(burnIn.Now.Sub(start).Minutes()), 0)
+	return start, burnIn.Now, expectedMinutes, true
 }
 
 // DeviceCriterion evaluates whether a device meets a specific readiness requirement.
@@ -73,7 +71,9 @@ func (e *DeviceHealthEvaluator) Evaluate(ctx context.Context, device serviceabil
 		return current
 	}
 
-	// Stage 1: Pending/Unknown → ReadyForLinks
+	// Stage 1: Pending/Unknown → ReadyForLinks.
+	// Evaluate advances at most one stage per call, so a device needs a minimum
+	// of two ticks (two worker intervals) to go from Pending to ReadyForUsers.
 	if current < serviceability.DeviceHealthReadyForLinks {
 		if !e.checkAll(ctx, device, e.ReadyForLinksCriteria) {
 			return current

controlplane/device-health-oracle/internal/worker/worker.go

@@ -75,7 +75,7 @@ func (w *Worker) tick(ctx context.Context) {
 		"drainedSlot", drainedSlot)
 
 	// Resolve burn-in boundary slots to wall-clock times for criteria evaluation.
-	var burnIn BurnInTimes
+	burnIn := BurnInTimes{Now: time.Now()}
 	if provisioningSlot > 0 {
 		bt, err := w.cfg.LedgerRPCClient.GetBlockTime(ctx, provisioningSlot)
 		if err != nil {