test(coverage): drive pool/circuit/telemetry/ctxkeys/root to ≥95%

internal/circuit/circuit_extra_test.go

@@ -0,0 +1,77 @@
+package circuit
+
+// circuit_extra_test.go — coverage for the constructor clamps, the nil-error
+// arm of isCallerDeadline, and the onOpen callback firing on a half-open
+// re-open (the existing suite uses NewBreaker without WithOnOpen, so that arm
+// was never exercised).
+
+import (
+	"testing"
+	"time"
+)
+
+// TestNewBreaker_ClampsBadConfig — a threshold < 1 snaps to 1 and a cooldown
+// <= 0 snaps to 30s, rather than panicking on the provision hot path.
+func TestNewBreaker_ClampsBadConfig(t *testing.T) {
+	// threshold 0 → clamps to 1: a single failure must open the breaker.
+	b := NewBreaker("clamp_threshold/"+t.Name(), 0, fastCooldown)
+	_ = b.Allow()
+	b.Record(errBoom)
+	if b.State() != StateOpen {
+		t.Fatalf("threshold<1 should clamp to 1 (one failure opens); state=%s", b.State())
+	}
+
+	// cooldown 0 → clamps to 30s: after opening, the breaker must stay open
+	// well past a few ms (proving the 30s default, not a zero cooldown that
+	// would immediately allow a trial).
+	b2 := NewBreaker("clamp_cooldown/"+t.Name(), 1, 0)
+	_ = b2.Allow()
+	b2.Record(errBoom)
+	if b2.State() != StateOpen {
+		t.Fatalf("expected open, got %s", b2.State())
+	}
+	time.Sleep(15 * time.Millisecond)
+	if b2.Allow() {
+		t.Fatal("cooldown<=0 should clamp to 30s — a trial must NOT be admitted after 15ms")
+	}
+}
+
+// TestIsCallerDeadline_Nil — the nil-error fast path returns false.
+func TestIsCallerDeadline_Nil(t *testing.T) {
+	if isCallerDeadline(nil) {
+		t.Fatal("isCallerDeadline(nil) = true, want false")
+	}
+}
+
+// TestRecord_HalfOpenReopen_FiresOnOpen — a half-open trial failure that
+// re-opens the breaker must fire the onOpen callback. The default suite never
+// installs onOpen, so this arm (the `if b.onOpen != nil` in the half-open
+// re-open path) was uncovered.
+func TestRecord_HalfOpenReopen_FiresOnOpen(t *testing.T) {
+	var opens int
+	b := NewBreaker("reopen_onopen/"+t.Name(), 1, fastCooldown).
+		WithOnOpen(func() { opens++ })
+
+	// Trip closed→open (fires onOpen once).
+	_ = b.Allow()
+	b.Record(errBoom)
+	if opens != 1 {
+		t.Fatalf("after first open, opens=%d want 1", opens)
+	}
+
+	// Cooldown, grab the half-open trial, fail it → re-open (fires onOpen again).
+	time.Sleep(shortWait)
+	if !b.Allow() {
+		t.Fatal("trial Allow() should succeed after cooldown")
+	}
+	if b.State() != StateHalfOpen {
+		t.Fatalf("expected half_open, got %s", b.State())
+	}
+	b.Record(errBoom)
+	if b.State() != StateOpen {
+		t.Fatalf("failed trial should re-open, got %s", b.State())
+	}
+	if opens != 2 {
+		t.Fatalf("half-open re-open should fire onOpen again; opens=%d want 2", opens)
+	}
+}

internal/ctxkeys/keys_test.go

@@ -0,0 +1,42 @@
+package ctxkeys
+
+// keys_test.go — round-trip coverage for the typed context key. The key's whole
+// purpose is collision-free storage of the owning team UUID on a context that
+// crosses package boundaries (gRPC handler → k8s backends), so the test asserts
+// (a) a value stored under TeamIDKey reads back unchanged, (b) a bare context
+// yields the zero value, and (c) the unexported keyed type does not collide with
+// a same-underlying-int key from another type.
+
+import (
+	"context"
+	"testing"
+)
+
+func TestTeamIDKey_RoundTrip(t *testing.T) {
+	const team = "team-abc-123"
+	ctx := context.WithValue(context.Background(), TeamIDKey, team)
+
+	got, ok := ctx.Value(TeamIDKey).(string)
+	if !ok {
+		t.Fatalf("value under TeamIDKey was not a string")
+	}
+	if got != team {
+		t.Fatalf("round-trip = %q, want %q", got, team)
+	}
+}
+
+func TestTeamIDKey_AbsentIsZero(t *testing.T) {
+	if v := context.Background().Value(TeamIDKey); v != nil {
+		t.Fatalf("bare context yielded %v under TeamIDKey, want nil", v)
+	}
+}
+
+// TestTeamIDKey_NoCollisionWithRawInt — the typed contextKey(0) must NOT alias a
+// plain int(0) key, which is the entire reason for the unexported named type.
+func TestTeamIDKey_NoCollisionWithRawInt(t *testing.T) {
+	ctx := context.WithValue(context.Background(), TeamIDKey, "scoped")
+	// A raw int key with the same underlying value must miss.
+	if v := ctx.Value(0); v != nil {
+		t.Fatalf("raw int(0) key collided with TeamIDKey: got %v", v)
+	}
+}

internal/pool/factory_test.go

@@ -0,0 +1,49 @@
+package pool
+
+// factory_test.go — NewWithConfig wires backend instances from a *config.Config
+// and hands them to New. It must construct a usable Manager without touching any
+// real infrastructure: the backend constructors are lazy (they don't dial until
+// a Provision call), so this is a pure wiring assertion.
+
+import (
+	"testing"
+
+	"github.com/jackc/pgx/v5/pgxpool"
+
+	"instant.dev/provisioner/internal/config"
+)
+
+// TestNewWithConfig_WiresBackends — NewWithConfig must return a non-nil Manager
+// with every backend slot populated and the targets map carrying the configured
+// sizes. A nil db is fine here; we never call a DB-touching method.
+func TestNewWithConfig_WiresBackends(t *testing.T) {
+	appCfg := &config.Config{
+		PostgresProvisionBackend: "local",
+		PostgresCustomersURL:     "postgres://nobody@127.0.0.1:1/postgres?sslmode=disable",
+		RedisProvisionBackend:    "local",
+		RedisProvisionHost:       "127.0.0.1:6379",
+		MongoProvisionBackend:    "local",
+		MongoAdminURI:            "mongodb://root:root@127.0.0.1:27017",
+		MongoHost:                "127.0.0.1:27017",
+		QueueProvisionBackend:    "local",
+		NATSHost:                 "127.0.0.1",
+	}
+	cfg := Config{PostgresSize: 3, RedisSize: 2, MongoSize: 1, QueueSize: 4}
+
+	var db *pgxpool.Pool // nil — NewWithConfig must not touch it
+	aesKey := make([]byte, 32)
+
+	m := NewWithConfig(db, aesKey, cfg, appCfg)
+	if m == nil {
+		t.Fatal("NewWithConfig returned nil")
+	}
+	if m.postgresB == nil || m.redisB == nil || m.mongoB == nil || m.queueB == nil {
+		t.Fatal("NewWithConfig left a backend slot nil")
+	}
+	wantTargets := map[string]int{"postgres": 3, "redis": 2, "mongodb": 1, "queue": 4}
+	for rt, want := range wantTargets {
+		if got := m.targets[rt]; got != want {
+			t.Errorf("targets[%q] = %d, want %d", rt, got, want)
+		}
+	}
+}

internal/pool/manager.go

@@ -94,8 +94,16 @@ type Manager struct {
 	// already tearing down (BugBash 2026-05-18 P3 — "pool shutdown ctx").
 	runCtx    context.Context
 	runCancel context.CancelFunc
+
+	// tickInterval is the period of the maintenance loop's health-check ticker.
+	// Zero means the production default (30s); a test sets a tiny value to
+	// exercise the periodic top-up arm without waiting 30 wall-clock seconds.
+	tickInterval time.Duration
 }
 
+// defaultTickInterval is the maintenance loop's periodic health-check period.
+const defaultTickInterval = 30 * time.Second
+
 // New creates a Manager. Call Start to begin background maintenance.
 func New(db *pgxpool.Pool, aesKey []byte, cfg Config,
 	postgresB postgres.Backend, redisB redis.Backend, mongoB mongo.Backend, queueB queue.Backend,
@@ -233,7 +241,11 @@ func (m *Manager) triggerRefill(resourceType string) {
 
 func (m *Manager) run(ctx context.Context) {
 	defer m.wg.Done()
-	ticker := time.NewTicker(30 * time.Second)
+	interval := m.tickInterval
+	if interval <= 0 {
+		interval = defaultTickInterval
+	}
+	ticker := time.NewTicker(interval)
 	defer ticker.Stop()
 
 	for {

internal/pool/manager_db_errors_test.go

@@ -0,0 +1,160 @@
+package pool
+
+// manager_db_errors_test.go — error-path coverage for the DB-touching seams
+// that the happy-path integration tests don't exercise: Claim's decrypt
+// failure, Stats / fillPool count-query failure (closed pool), and the queue
+// resource type through provisionItemsConcurrently. Same TEST_PROVISIONER_-
+// DATABASE_URL gate as manager_db_test.go.
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/jackc/pgx/v5/pgxpool"
+)
+
+// TestClaim_DecryptFailure — a ready row whose connection_url is not valid
+// ciphertext must make Claim return a decrypt error (not panic, not a bogus
+// item). crypto.Decrypt fails-open in the resource read path elsewhere, but in
+// pool.Claim a malformed stored URL is a hard error — surface it.
+func TestClaim_DecryptFailure(t *testing.T) {
+	m, pool, _ := newDBManager(t, Config{})
+
+	// Insert a ready row directly with a connection_url that is not a valid
+	// AES-GCM ciphertext for m.aesKey.
+	if _, err := pool.Exec(context.Background(), `
+		INSERT INTO pool_items (resource_type, connection_url, pool_token, status)
+		VALUES ('postgres', 'not-valid-ciphertext', 'pool-bad', 'ready')
+	`); err != nil {
+		t.Fatalf("seed bad row: %v", err)
+	}
+
+	item, err := m.Claim(context.Background(), "postgres")
+	if err == nil {
+		t.Fatalf("Claim should error on undecryptable url; got item=%+v", item)
+	}
+	if item != nil {
+		t.Fatalf("Claim returned a non-nil item on decrypt failure: %+v", item)
+	}
+}
+
+// TestStats_QueryError — Stats over a closed pool must return an error rather
+// than an empty map. Exercises the fmt.Errorf("pool.Stats: ...") arm.
+func TestStats_QueryError(t *testing.T) {
+	dsn := testDSN(t)
+	pool, err := pgxpool.New(context.Background(), dsn)
+	if err != nil {
+		t.Fatalf("pgxpool.New: %v", err)
+	}
+	m := New(pool, make([]byte, 32), Config{}, nil, nil, nil, nil)
+	if err := m.migrate(context.Background()); err != nil {
+		t.Fatalf("migrate: %v", err)
+	}
+	pool.Close() // now every query fails
+
+	if _, err := m.Stats(context.Background()); err == nil {
+		t.Fatal("Stats over a closed pool should return an error")
+	}
+}
+
+// TestFillPool_CountError — fillPool over a closed pool must hit the count-query
+// error arm and return without provisioning (and without panicking).
+func TestFillPool_CountError(t *testing.T) {
+	dsn := testDSN(t)
+	pool, err := pgxpool.New(context.Background(), dsn)
+	if err != nil {
+		t.Fatalf("pgxpool.New: %v", err)
+	}
+	tr := &concurrencyTracker{}
+	m := New(pool, make([]byte, 32), Config{PostgresSize: 2},
+		&mockPostgresBackend{tr: tr}, &mockRedisBackend{tr: tr},
+		&mockMongoBackend{tr: tr}, &mockQueueBackend{tr: tr})
+	if err := m.migrate(context.Background()); err != nil {
+		t.Fatalf("migrate: %v", err)
+	}
+	pool.Close()
+
+	// Must return cleanly (count query errors, logs, returns) — no provision.
+	m.fillPool(context.Background(), "postgres")
+	if tr.Peak() != 0 {
+		t.Fatalf("fillPool provisioned %d items despite count error; want 0", tr.Peak())
+	}
+}
+
+// TestFillPool_QueueType — drives the queue arm of provisionOneItemBackend +
+// the INSERT through fillPool against a real DB, closing the last
+// provisionOneItemBackend switch branch the postgres/redis/mongo tests miss.
+func TestFillPool_QueueType(t *testing.T) {
+	m, pool, _ := newDBManager(t, Config{QueueSize: 2})
+	m.fillPool(context.Background(), "queue")
+	if got := readyCount(t, pool, "queue"); got != 2 {
+		t.Fatalf("ready queue count = %d, want 2", got)
+	}
+}
+
+// TestStart_PeriodicTickRefill — Start launches the maintenance loop whose
+// ticker branch periodically tops up every type. We can't wait the real 30s
+// tick, but we can prove the loop is alive by triggering a refill via Claim's
+// async triggerRefill and confirming the pool re-fills after a claim drains it.
+// This exercises the run() refillCh arm beyond the single initial fill.
+func TestStart_PeriodicTickRefill(t *testing.T) {
+	m, pool, _ := newDBManager(t, Config{RedisSize: 2})
+	if err := m.Start(context.Background()); err != nil {
+		t.Fatalf("Start: %v", err)
+	}
+	t.Cleanup(m.Shutdown)
+
+	// Wait for initial fill to reach target.
+	waitFor(t, func() bool { return readyCount(t, pool, "redis") == 2 }, 5*time.Second,
+		"initial refill never reached target 2")
+
+	// Claim one — this fires triggerRefill, so run()'s refillCh arm tops it back up.
+	item, err := m.Claim(context.Background(), "redis")
+	if err != nil || item == nil {
+		t.Fatalf("Claim: item=%v err=%v", item, err)
+	}
+
+	// The async refill triggered by Claim must restore the target.
+	waitFor(t, func() bool { return readyCount(t, pool, "redis") == 2 }, 5*time.Second,
+		"pool did not re-fill to target after a claim drained it")
+}
+
+// TestRun_PeriodicTickFills — the maintenance loop's ticker.C arm must top up
+// every type without an explicit triggerRefill. We set a tiny tickInterval,
+// drain the pool by deleting its ready rows out-of-band (no triggerRefill
+// fired), and assert the periodic tick refills it back to target.
+func TestRun_PeriodicTickFills(t *testing.T) {
+	m, pool, _ := newDBManager(t, Config{RedisSize: 2})
+	m.tickInterval = 30 * time.Millisecond // fast ticks for the test
+
+	if err := m.Start(context.Background()); err != nil {
+		t.Fatalf("Start: %v", err)
+	}
+	t.Cleanup(m.Shutdown)
+
+	waitFor(t, func() bool { return readyCount(t, pool, "redis") == 2 }, 5*time.Second,
+		"initial refill never reached target")
+
+	// Drain out-of-band: delete ready rows directly so NO triggerRefill is
+	// queued. Only the periodic ticker can bring the pool back.
+	if _, err := pool.Exec(context.Background(),
+		`DELETE FROM pool_items WHERE resource_type='redis'`); err != nil {
+		t.Fatalf("drain: %v", err)
+	}
+
+	waitFor(t, func() bool { return readyCount(t, pool, "redis") == 2 }, 5*time.Second,
+		"periodic ticker did not re-fill the pool after an out-of-band drain")
+}
+
+// waitFor polls cond until true or the deadline, failing with msg on timeout.
+func waitFor(t *testing.T, cond func() bool, d time.Duration, msg string) {
+	t.Helper()
+	deadline := time.Now().Add(d)
+	for !cond() {
+		if time.Now().After(deadline) {
+			t.Fatal(msg)
+		}
+		time.Sleep(20 * time.Millisecond)
+	}
+}