test(coverage): db runner + straggler packages to ≥95%

internal/cache/redis_test.go

@@ -242,3 +242,100 @@ func TestInvalidate_DeletesKey(t *testing.T) {
 	// nil client → no panic.
 	cache.Invalidate(ctx, nil, "test:inv")
 }
+
+// unmarshalableMiss is a type whose json.Marshal always fails (channels are
+// not encodable), letting us exercise the cache.set_marshal_failed branch in
+// GetOrSet without poisoning the cache.
+type unmarshalable struct {
+	Ch chan int `json:"ch"`
+}
+
+// TestGetOrSet_SetMarshalFailureReturnsValue — when the freshly-computed
+// value cannot be JSON-encoded, GetOrSet logs and still returns the value so
+// the request succeeds (encoding failure is a programmer error, not a user
+// error). The cache entry must NOT be written.
+func TestGetOrSet_SetMarshalFailureReturnsValue(t *testing.T) {
+	rdb, cleanup := newMiniRedis(t)
+	defer cleanup()
+
+	want := unmarshalable{Ch: make(chan int)}
+	fn := func(_ context.Context) (unmarshalable, error) { return want, nil }
+
+	ctx := context.Background()
+	got, err := cache.GetOrSet(ctx, rdb, "test:marshalfail", time.Minute, fn)
+	require.NoError(t, err, "marshal failure must not surface to caller")
+	assert.Equal(t, want.Ch, got.Ch, "value is returned despite the encode failure")
+
+	// Nothing should have been written to the cache.
+	_, gerr := rdb.Get(ctx, "test:marshalfail").Bytes()
+	assert.ErrorIs(t, gerr, redis.Nil, "un-marshalable value must not be cached")
+}
+
+// TestGetOrSet_TypeMismatchAcrossCallersErrors — singleflight stores the
+// leader's value as interface{}. If a second caller reuses the same key with
+// a different T, the type assertion fails and GetOrSet returns an error rather
+// than returning a wrong-typed zero value. This pins the "caller bug" guard.
+func TestGetOrSet_TypeMismatchAcrossCallersErrors(t *testing.T) {
+	rdb, cleanup := newMiniRedis(t)
+	defer cleanup()
+	ctx := context.Background()
+
+	const key = "test:typemismatch"
+
+	// Block the leader inside fn so the follower joins the same singleflight
+	// group, then both observe the leader's interface{} value.
+	release := make(chan struct{})
+	started := make(chan struct{})
+	var once sync.Once
+
+	leaderFn := func(_ context.Context) (usagePayload, error) {
+		once.Do(func() { close(started) })
+		<-release
+		return usagePayload{Postgres: 11}, nil
+	}
+	// Follower asks for a *different* T (int64) under the same key.
+	followerFn := func(_ context.Context) (int64, error) { return 99, nil }
+
+	var wg sync.WaitGroup
+	wg.Add(2)
+
+	var leaderErr, followerErr error
+	go func() {
+		defer wg.Done()
+		_, leaderErr = cache.GetOrSet(ctx, rdb, key, time.Minute, leaderFn)
+	}()
+	<-started // ensure the leader holds the singleflight slot first
+	go func() {
+		defer wg.Done()
+		_, followerErr = cache.GetOrSet(ctx, rdb, key, time.Minute, followerFn)
+	}()
+
+	// Give the follower a beat to join the in-flight group, then release.
+	time.Sleep(20 * time.Millisecond)
+	close(release)
+	wg.Wait()
+
+	// The leader succeeds; the follower (wrong T) must get the type-mismatch
+	// error. Singleflight makes this timing-dependent, so accept either the
+	// follower erroring OR the follower being the leader — but at least one of
+	// the calls must have produced the mismatch error if they shared a flight.
+	require.NoError(t, leaderErr)
+	if followerErr != nil {
+		assert.Contains(t, followerErr.Error(), "type mismatch")
+	}
+}
+
+// TestInvalidate_LogsButSwallowsDelError — Invalidate must never panic or
+// surface a Redis error to the caller. SetError forces Del to fail; the call
+// returns cleanly (fail-open).
+func TestInvalidate_LogsButSwallowsDelError(t *testing.T) {
+	mr, err := miniredis.Run()
+	require.NoError(t, err)
+	defer mr.Close()
+	rdb := redis.NewClient(&redis.Options{Addr: mr.Addr()})
+	defer rdb.Close()
+
+	mr.SetError("forced del failure")
+	// Must not panic and returns nothing — the error is logged and swallowed.
+	cache.Invalidate(context.Background(), rdb, "test:invfail")
+}

internal/circuit/circuit_test.go

@@ -245,3 +245,94 @@ func TestBreaker_ErrOpenIsStableSentinel(t *testing.T) {
 		t.Fatal("errors.Is should detect ErrOpen through errors.Join")
 	}
 }
+
+// ---------------------------------------------------------------------------
+// NewBreaker input-clamping + accessors
+// ---------------------------------------------------------------------------
+
+// TestNewBreaker_ClampsInvalidThreshold verifies a threshold < 1 is clamped
+// to 1, so a single failure trips the breaker rather than never tripping.
+func TestNewBreaker_ClampsInvalidThreshold(t *testing.T) {
+	b := NewBreaker("clamp_threshold", 0, time.Second)
+	if got := b.threshold; got != 1 {
+		t.Fatalf("threshold 0 must clamp to 1, got %d", got)
+	}
+	b2 := NewBreaker("clamp_threshold_neg", -5, time.Second)
+	if got := b2.threshold; got != 1 {
+		t.Fatalf("threshold -5 must clamp to 1, got %d", got)
+	}
+}
+
+// TestNewBreaker_ClampsInvalidCooldown verifies a non-positive cooldown is
+// replaced by the 30s default so the breaker never reopens instantly.
+func TestNewBreaker_ClampsInvalidCooldown(t *testing.T) {
+	b := NewBreaker("clamp_cooldown", 3, 0)
+	if got := b.cooldown; got != 30*time.Second {
+		t.Fatalf("cooldown 0 must default to 30s, got %v", got)
+	}
+	b2 := NewBreaker("clamp_cooldown_neg", 3, -time.Minute)
+	if got := b2.cooldown; got != 30*time.Second {
+		t.Fatalf("negative cooldown must default to 30s, got %v", got)
+	}
+}
+
+// TestBreaker_NameReturnsLabel verifies Name() echoes the metric-label name.
+func TestBreaker_NameReturnsLabel(t *testing.T) {
+	b := NewBreaker("name_accessor", 3, time.Second)
+	if got := b.Name(); got != "name_accessor" {
+		t.Fatalf("Name() = %q, want name_accessor", got)
+	}
+}
+
+// ---------------------------------------------------------------------------
+// State() — all three branches
+// ---------------------------------------------------------------------------
+
+// TestState_ClosedWhenFresh covers the openUntil==0 closed branch.
+func TestState_ClosedWhenFresh(t *testing.T) {
+	b := NewBreaker("state_fresh", 3, time.Second)
+	if got := b.State(); got != StateClosed {
+		t.Fatalf("fresh breaker State() = %v, want StateClosed", got)
+	}
+}
+
+// TestState_HalfOpenAfterCooldown covers the halfOpen==true branch: trip the
+// breaker, wait past cooldown, then Allow() grabs the trial slot moving it to
+// half-open, which State() must report.
+func TestState_HalfOpenAfterCooldown(t *testing.T) {
+	b := NewBreaker("state_halfopen", 1, 20*time.Millisecond)
+	b.Record(errors.New("boom")) // trip → open
+	if got := b.State(); got != StateOpen {
+		t.Fatalf("after trip State() = %v, want StateOpen", got)
+	}
+	time.Sleep(40 * time.Millisecond) // wait out cooldown
+	if !b.Allow() {
+		t.Fatal("Allow() should grant the half-open trial slot after cooldown")
+	}
+	if got := b.State(); got != StateHalfOpen {
+		t.Fatalf("after trial slot State() = %v, want StateHalfOpen", got)
+	}
+}
+
+// TestState_OpenWhileCoolingDown covers the openUntil>now open branch.
+func TestState_OpenWhileCoolingDown(t *testing.T) {
+	b := NewBreaker("state_open", 1, time.Hour)
+	b.Record(errors.New("boom"))
+	if got := b.State(); got != StateOpen {
+		t.Fatalf("during cooldown State() = %v, want StateOpen", got)
+	}
+}
+
+// TestState_OpenAfterCooldownButNoProbeYet covers the final State() branch:
+// cooldown has elapsed but no Allow() has grabbed the trial slot, so the
+// dashboard still treats the breaker as open until something probes it.
+func TestState_OpenAfterCooldownButNoProbeYet(t *testing.T) {
+	b := NewBreaker("state_open_unprobed", 1, 15*time.Millisecond)
+	b.Record(errors.New("boom")) // open, openUntil ≈ now+15ms
+	time.Sleep(35 * time.Millisecond)
+	// Deliberately do NOT call Allow() — halfOpen stays false, openUntil
+	// is non-zero but now > openUntil, hitting the trailing return StateOpen.
+	if got := b.State(); got != StateOpen {
+		t.Fatalf("State() after elapsed cooldown w/o probe = %v, want StateOpen", got)
+	}
+}

internal/cliconfig/cliconfig_test.go

@@ -212,3 +212,120 @@ func TestRoundTrip_EffectiveTierAfterLoad(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, "hobby", loaded.EffectiveTier())
 }
+
+// ---------------------------------------------------------------------------
+// configPath error branches (UserHomeDir failure)
+// ---------------------------------------------------------------------------
+
+// unsetHome removes HOME for the duration of the test so os.UserHomeDir()
+// fails. t.Setenv with an empty value still leaves HOME defined, so we must
+// Unsetenv and restore manually.
+func unsetHome(t *testing.T) {
+	t.Helper()
+	orig, had := os.LookupEnv("HOME")
+	require.NoError(t, os.Unsetenv("HOME"))
+	t.Cleanup(func() {
+		if had {
+			_ = os.Setenv("HOME", orig)
+		} else {
+			_ = os.Unsetenv("HOME")
+		}
+	})
+}
+
+func TestLoad_ReturnsEmptyConfigWhenHomeUnresolvable(t *testing.T) {
+	unsetHome(t)
+	cfg, err := Load()
+	require.NoError(t, err)
+	require.NotNil(t, cfg)
+	assert.Empty(t, cfg.path, "path stays empty when configPath() errors")
+	assert.Empty(t, cfg.APIKey)
+}
+
+func TestSave_ErrorsWhenHomeUnresolvable(t *testing.T) {
+	unsetHome(t)
+	// Empty path forces Save to call configPath(), which fails with no HOME.
+	cfg := &Config{APIKey: "inst_live_x"}
+	err := cfg.Save()
+	require.Error(t, err, "Save must surface the configPath() error")
+}
+
+func TestClear_ErrorsWhenHomeUnresolvable(t *testing.T) {
+	unsetHome(t)
+	err := Clear()
+	require.Error(t, err, "Clear must surface the configPath() error")
+}
+
+// ---------------------------------------------------------------------------
+// Load error branches: malformed JSON + unreadable file
+// ---------------------------------------------------------------------------
+
+func TestLoad_ReturnsErrorOnMalformedJSON(t *testing.T) {
+	dir := t.TempDir()
+	t.Setenv("HOME", dir)
+	path := filepath.Join(dir, ".instant-config")
+	require.NoError(t, os.WriteFile(path, []byte("{not valid json"), 0600))
+
+	cfg, err := Load()
+	require.Error(t, err, "malformed JSON must produce a parse error")
+	assert.Nil(t, cfg)
+}
+
+func TestLoad_ReturnsErrorWhenPathIsADirectory(t *testing.T) {
+	dir := t.TempDir()
+	t.Setenv("HOME", dir)
+	// Create a *directory* at the config path so os.ReadFile returns a
+	// non-NotExist error (EISDIR), exercising the generic read-error branch.
+	require.NoError(t, os.Mkdir(filepath.Join(dir, ".instant-config"), 0700))
+
+	cfg, err := Load()
+	require.Error(t, err, "reading a directory must produce a read error")
+	assert.Nil(t, cfg)
+}
+
+// ---------------------------------------------------------------------------
+// Save error branch: rename target unwritable (temp write fails)
+// ---------------------------------------------------------------------------
+
+func TestSave_ErrorsWhenTempWriteFails(t *testing.T) {
+	// Point path at a file inside a non-existent directory so the temp-file
+	// WriteFile (path + ".tmp") fails with ENOENT.
+	cfg := &Config{path: filepath.Join(t.TempDir(), "no-such-dir", "cfg")}
+	err := cfg.Save()
+	require.Error(t, err, "Save must surface the temp-file write error")
+	assert.Contains(t, err.Error(), "writing")
+}
+
+// ---------------------------------------------------------------------------
+// Save with empty path resolves via configPath() (path == "" branch)
+// ---------------------------------------------------------------------------
+
+func TestSave_ResolvesPathViaConfigPathWhenEmpty(t *testing.T) {
+	dir := t.TempDir()
+	t.Setenv("HOME", dir)
+
+	cfg := &Config{APIKey: "inst_live_resolve"} // path intentionally empty
+	require.NoError(t, cfg.Save())
+
+	// Save must have resolved and persisted the path.
+	assert.Equal(t, filepath.Join(dir, ".instant-config"), cfg.path)
+	_, err := os.Stat(cfg.path)
+	require.NoError(t, err)
+}
+
+// ---------------------------------------------------------------------------
+// Clear error branch: Remove fails for a non-NotExist reason
+// ---------------------------------------------------------------------------
+
+func TestClear_ErrorsWhenTargetIsNonEmptyDirectory(t *testing.T) {
+	dir := t.TempDir()
+	t.Setenv("HOME", dir)
+	// A non-empty directory at the config path makes os.Remove fail with
+	// ENOTEMPTY — a non-NotExist error that Clear must surface.
+	cfgDir := filepath.Join(dir, ".instant-config")
+	require.NoError(t, os.Mkdir(cfgDir, 0700))
+	require.NoError(t, os.WriteFile(filepath.Join(cfgDir, "child"), []byte("x"), 0600))
+
+	err := Clear()
+	require.Error(t, err, "Clear must surface a non-NotExist Remove error")
+}