fix(registry): make the breaker actually suppress re-probing

Address the design finding and remaining defects from round 3 of
adversarial review.

Raise defaultBreakerMaxOpenDuration from 30m to 6h. Round 2 fixed the
breaker so it never retries more than plain interval throttling, but
left it cadence-neutral: with maxOpenDuration shorter than the 1h
default interval, the interval always dominated and backoff never
suppressed anything. Measured over 24h with stock config, admits
equaled the interval-only baseline exactly. At 6h, a permanently broken
provider now backs off 1h, 2h, 4h, 6h, 6h, roughly 5 attempts a day
instead of 24. The 24h cadence test now asserts admits are strictly
fewer than baseline, not merely no worse.

Correct the emitted openUntil to account for the interval floor.
Because the interval gate applies unconditionally, the real next probe
time is max(openUntil, lastRefresh+interval), not the raw backoff value
alone. On pure defaults a fresh trip set the raw value to +1m while the
real next probe was at +1h, a 59 minute skew the dashboard rendered as
a countdown that reached zero and then silently vanished. snapshot now
emits the corrected value while open; closed and half-open are
unaffected.

Revert a canceled half-open probe to open instead of leaving it at
half-open. A half-open breaker reports healthy by design, so a canceled
probe previously left a still-broken provider reading healthy until the
next attempt happened to land. Reverting costs nothing extra, since the
interval gate already governs when that next attempt is admitted either
way.

Rewrite the discovery-suppression claims in doc comments: the earlier
wording claimed re-probing stops "every single interval forever" under
all conditions, which the interval-floor fix from round 2 already made
false, and which contradicted this file's own surrounding explanation.
Both now state plainly that suppression only occurs once backoff
exceeds discoveryInterval.

Add a sentence to recordHealthLocked's doc comment stating that
context.DeadlineExceeded counts as a plain failure, matching
recordProviderAttempt's SHOULD-1 ruling, and a named test proving it.

Tests: two new snapshot tests pin the openUntil correction in both
directions. The cadence test's baseline comparison is now strict. A new
test proves the zero value truly disables the breaker rather than
tripping on the first failure. A new test proves a canceled context
never gets misclassified as a failure, completing the bidirectional
check the existing success-direction test already had. The concurrent
test now asserts backoff stays within its ceiling and health stays a
valid state on every iteration, rather than only checking for panics
and data races.

registry.go

@@ -47,8 +47,18 @@ const defaultBreakerOpenDuration = time.Minute
 
 // defaultBreakerMaxOpenDuration caps the exponential backoff the breaker
 // doubles into on every further failed probe, applied when BreakerConfig.
-// MaxOpenDuration is left empty.
-const defaultBreakerMaxOpenDuration = 30 * time.Minute
+// MaxOpenDuration is left empty. Deliberately LONGER than
+// defaultDiscoveryInterval (1h) — design ruling, adversarial review round
+// 3: a breaker whose backoff can never exceed the plain interval cannot
+// suppress anything, since tryBeginRefresh's interval gate (see its own
+// doc comment) already applies unconditionally and only the LONGER of the
+// two ever binds. An earlier value here (30m, shorter than the 1h
+// interval) made the breaker cadence-neutral under stock config — exactly
+// as strict an interval-only gate would already produce, with none of the
+// suppression this feature exists to provide. At 6h, a permanently broken
+// provider under every default backs off 1h, 2h, 4h, 6h, 6h, ... — roughly
+// 5 attempts a day instead of 24.
+const defaultBreakerMaxOpenDuration = 6 * time.Hour
 
 // maxBreakerOpenDuration bounds BreakerConfig.MaxOpenDuration — the same
 // ceiling-on-a-config-value precedent maxBreakerFailureThreshold already
@@ -240,23 +250,32 @@ func (st *providerState) setDiscoveredContext(m map[string]int) {
 // call.
 //
 // The interval gate (now far enough past lastRefresh, or this is the first
-// refresh) applies UNCONDITIONALLY, closed or open alike — this is the
-// fix for the adversarial-review blocker (round 2): an EARLIER version of
-// this method let an open breaker's own backoff window (openUntil) REPLACE
-// the interval gate rather than add to it, and since
-// defaultBreakerMaxOpenDuration (30m) is shorter than defaultDiscoveryInterval
-// (1h), that let a permanently broken provider get probed roughly TWICE as
-// often as before this feature existed — the exact opposite of "back off".
-// Backoff can only ever make retries LESS frequent than the plain interval,
-// never more: a closed breaker is gated by interval alone (exactly as
-// before this feature existed — a provider that never fails never touches
-// the breaker branch below at all); an open breaker is additionally gated
-// by openUntil, so whichever of the two — interval or backoff — is
-// currently longer wins. Reaching both starts exactly one half-open probe
-// (state moves to halfOpen here, before the caller's fetch even runs)
-// rather than resuming normal per-interval refreshing outright —
+// refresh) applies UNCONDITIONALLY, closed or open alike — fix for the
+// adversarial-review blocker (round 2): an EARLIER version of this method
+// let an open breaker's own backoff window (openUntil) REPLACE the
+// interval gate rather than add to it. Since that round's default
+// maxOpenDuration was shorter than the default interval, a permanently
+// broken provider got probed roughly TWICE as often as before this
+// feature existed — the exact opposite of "back off". Backoff can only
+// ever make retries LESS frequent than the plain interval, never more: a
+// closed breaker is gated by interval alone (exactly as before this
+// feature existed — a provider that never fails never touches the
+// breaker branch below at all); an open breaker is additionally gated by
+// openUntil, so whichever of the two — interval or backoff — is currently
+// longer wins. Reaching both starts exactly one half-open probe (state
+// moves to halfOpen here, before the caller's fetch even runs) rather
+// than resuming normal per-interval refreshing outright —
 // recordHealthLocked decides whether that probe closes the breaker or
 // reopens it with a longer backoff.
+//
+// PRACTICAL CONSEQUENCE, design ruling (round 3): whichever of interval
+// and maxOpenDuration is larger is the one that actually governs a
+// persistently broken provider's re-probe cadence — the smaller of the
+// two is masked entirely, never merely "adds a little more delay". A
+// deployment whose maxOpenDuration does not exceed its discoveryInterval
+// gets NO suppression benefit from this feature at all; see
+// defaultBreakerMaxOpenDuration's own doc comment for why the shipped
+// defaults are chosen to avoid exactly that trap.
 func (st *providerState) tryBeginRefresh(now time.Time) bool {
 	st.mu.Lock()
 	defer st.mu.Unlock()
@@ -317,14 +336,29 @@ func (st *providerState) finishRefresh(now time.Time, ids []string, err error) {
 // method had — must not count as a SUCCESS either: treating it as success
 // would CLOSE an already-open breaker and wipe its backoff/counters purely
 // because a half-open probe got interrupted before it could complete,
-// reporting a still-broken provider as healthy again. Every field this
-// method could touch is left exactly as it was; only lastRefresh/lastErr
-// (finishRefresh, above, unconditionally) and inFlight change. If health
-// is still halfOpen afterward, the NEXT real attempt (admitted immediately
-// — see tryBeginRefresh's own doc comment: a halfOpen provider is not
-// gated by openUntil at all, only by the plain interval) re-decides the
-// breaker through this same function, so a canceled probe simply delays
-// the decision by one cycle rather than making a wrong one now.
+// reporting a still-broken provider as healthy again. Every OTHER field
+// this method could touch is left exactly as it was; only lastRefresh/
+// lastErr (finishRefresh, above, unconditionally) and inFlight change.
+//
+// A canceled probe while halfOpen is the one exception (round 3, MEDIUM
+// finding): health reverts to OPEN with its backoff/openUntil left
+// completely untouched, rather than staying at halfOpen. discoveryHealthy
+// reports true for halfOpen (by design — a provider actively being
+// re-probed must stay routable), so leaving a canceled probe's state at
+// halfOpen indefinitely would report a still-broken provider healthy
+// until the next attempt happens to land, which the plain interval gate
+// alone does not bound tightly. Reverting to open costs nothing extra —
+// tryBeginRefresh's own interval gate already runs before this is ever
+// reached, so the NEXT attempt is admitted at the identical instant
+// either way; only what discoveryHealthy reports in the meantime differs.
+//
+// context.DeadlineExceeded is NOT given the same neutral treatment as
+// Canceled — deliberately: it counts as a plain failure, below, matching
+// recordProviderAttempt's own SHOULD-1 ruling (limits.go) that a deadline
+// the GATEWAY set and the upstream never answered inside is a genuine
+// provider-health signal, not a caller-walked-away one. Three consecutive
+// backgroundRefreshTimeout (30s) overruns trip the breaker exactly like
+// three plain errors would.
 //
 // failed classifies every OTHER error explicitly for provider health,
 // deliberately NOT reusing isTransient's retry-shaped semantics (retry.go)
@@ -352,12 +386,20 @@ func (st *providerState) finishRefresh(now time.Time, ids []string, err error) {
 // While halfOpen (the only other state finishRefresh can observe —
 // tryBeginRefresh never starts a refresh from open without first moving to
 // halfOpen), a successful probe closes the breaker outright; a failed one
-// reopens it with a DOUBLED backoff, capped at breakerOpenMax — the
-// exponential escalation that keeps a persistently broken provider (like
-// xiaomi's ongoing 401s) from being re-probed every single interval
-// forever.
+// reopens it with a DOUBLED backoff, capped at breakerOpenMax. This ONLY
+// suppresses re-probing when the resulting backoff exceeds discoveryInterval
+// — design ruling, round 3: tryBeginRefresh's interval gate (its own doc
+// comment) applies regardless of breaker state, so a maxOpenDuration
+// shorter than discoveryInterval is masked entirely and this escalation
+// changes nothing observable. defaultBreakerMaxOpenDuration is deliberately
+// set longer than defaultDiscoveryInterval so this DOES suppress re-probing
+// under stock config — see that constant's own doc comment for the
+// resulting cadence.
 func (st *providerState) recordHealthLocked(now time.Time, err error) {
 	if errors.Is(err, context.Canceled) {
+		if st.health == breakerHalfOpen {
+			st.health = breakerOpen
+		}
 		return
 	}
 	failed := err != nil
@@ -451,10 +493,30 @@ func (st *providerState) discoveryHealthy() bool {
 // lastRefresh/lastErr: a concurrent finishRefresh must never be able to
 // hand the admin dashboard a breaker state that disagrees with the
 // lastErr/models it is shown alongside.
+//
+// openUntil is NOT returned raw (round 3 MEDIUM finding): since
+// tryBeginRefresh's interval gate applies unconditionally (its own doc
+// comment), the REAL next probe time is max(st.openUntil,
+// lastRefresh+interval), which can differ sharply from st.openUntil alone
+// — under stock defaults, a fresh trip sets st.openUntil to +1m while the
+// first admitted probe is actually at +1h, a 59-minute skew an operator
+// watching the dashboard would otherwise see as a broken countdown
+// (formatUntil renders "in 60s", nothing happens, then the timestamp goes
+// stale and the countdown vanishes for the remaining 59 minutes). This
+// correction only ever applies while OPEN — st.openUntil is always the
+// zero time while closed (closeBreakerLocked) or a past instant while
+// half-open (a probe only ever starts once now has already reached it),
+// neither of which this dashboard-facing value should be adjusted for.
 func (st *providerState) snapshot() (models []string, lastRefresh time.Time, lastErr string, health breakerState, openUntil time.Time) {
 	st.mu.Lock()
 	defer st.mu.Unlock()
-	return st.knownIDsLocked(), st.lastRefresh, st.lastErr, st.health, st.openUntil
+	effectiveOpenUntil := st.openUntil
+	if st.health == breakerOpen {
+		if floor := st.lastRefresh.Add(st.interval); floor.After(effectiveOpenUntil) {
+			effectiveOpenUntil = floor
+		}
+	}
+	return st.knownIDsLocked(), st.lastRefresh, st.lastErr, st.health, effectiveOpenUntil
 }
 
 // modelRegistry aggregates every configured provider's explicit and

registry_health_test.go

@@ -201,6 +201,28 @@ func TestProviderState_Breaker_ErrRequestBuildFailed_CountsAsFailure(t *testing.
 	}
 }
 
+// TestProviderState_Breaker_ContextDeadlineExceeded_CountsAsFailure proves
+// context.DeadlineExceeded is a plain failure, NOT given Canceled's
+// neutral treatment — deliberate, matching recordProviderAttempt's own
+// SHOULD-1 ruling (limits.go): a deadline the GATEWAY set and the upstream
+// never answered inside is a genuine provider-health signal, unlike a
+// caller-canceled context. Three consecutive backgroundRefreshTimeout
+// (30s) overruns against a merely slow provider trip the breaker exactly
+// like three plain errors would.
+func TestProviderState_Breaker_ContextDeadlineExceeded_CountsAsFailure(t *testing.T) {
+	t.Parallel()
+	st := newHealthTestState(time.Hour, 3, time.Minute, 10*time.Minute)
+	now := time.Now()
+
+	for i := 0; i < 3; i++ {
+		st.finishRefresh(now, nil, context.DeadlineExceeded)
+	}
+
+	if st.discoveryHealthy() {
+		t.Fatal("discoveryHealthy() = true after 3 consecutive context.DeadlineExceeded outcomes, want false — a deadline the gateway set and the upstream never answered inside is a provider fault, not a neutral cancellation")
+	}
+}
+
 // --- breaker: the zero value never trips (adversarial-review finding,
 // round 2 — providerState's own doc comment claims "zero value behaves
 // exactly as before this feature existed"; without this guard that claim
@@ -234,29 +256,39 @@ func TestProviderState_Breaker_ZeroThreshold_NeverOpens(t *testing.T) {
 	}
 }
 
-// --- breaker: discovery backoff must never retry MORE often than plain
-// interval throttling (BLOCKING adversarial-review finding, round 2) ---
+// --- breaker: discovery backoff must actually suppress re-probing under
+// DEFAULT config, strictly, not merely stay no-worse-than plain interval
+// throttling (BLOCKING adversarial-review findings, rounds 2 and 3) ---
 
-// TestProviderState_Breaker_DefaultCadence_NoWorseThanPreFeatureBaseline
-// simulates 24 hours of a permanently-failing provider under every
-// DEFAULT setting (the shipped configuration, the xiaomi case this
-// feature exists for) and counts actual ADMITTED refresh attempts.
+// TestProviderState_Breaker_DefaultCadence_StrictlyFewerThanPreFeatureBaseline
+// simulates 24 hours of a permanently-failing provider under every DEFAULT
+// setting (the shipped configuration, the xiaomi case this feature exists
+// for) and counts actual ADMITTED refresh attempts.
 //
-// An earlier version of tryBeginRefresh let an open breaker's own backoff
-// window (openUntil) REPLACE the plain interval gate instead of adding to
-// it. Since defaultBreakerMaxOpenDuration (30m) is shorter than
-// defaultDiscoveryInterval (1h), that measured 50 admitted attempts here —
-// MORE than the 24 a plain, breaker-less interval gate would ever produce
-// for the same 24h window. For a permanently broken provider that means
-// MORE upstream 401s and MORE ERROR lines than today, the exact inverse of
-// this feature's stated purpose. tryBeginRefresh now applies the interval
-// gate unconditionally (closed or open alike), so backoff can only ever
-// make retries LESS frequent, never more — and because
-// defaultBreakerMaxOpenDuration is itself shorter than
-// defaultDiscoveryInterval, the interval remains the sole binding
-// constraint under pure defaults, so this asserts EQUALITY with the
-// baseline, not just "no worse".
-func TestProviderState_Breaker_DefaultCadence_NoWorseThanPreFeatureBaseline(t *testing.T) {
+// Round 2's earlier version of tryBeginRefresh let an open breaker's own
+// backoff window (openUntil) REPLACE the plain interval gate instead of
+// adding to it. Since that round's defaultBreakerMaxOpenDuration (30m) was
+// shorter than defaultDiscoveryInterval (1h), that measured 50 admitted
+// attempts here — MORE than the 24 a plain, breaker-less interval gate
+// would ever produce for the same 24h window: the exact inverse of this
+// feature's purpose. The round-2 fix (interval gate applies
+// unconditionally) closed that regression, but left the feature CADENCE-
+// NEUTRAL: with defaultBreakerMaxOpenDuration still shorter than
+// defaultDiscoveryInterval, the interval alone remained the sole binding
+// constraint and this test measured EXACTLY 24 — no worse than baseline,
+// but no better either, doing nothing a plain interval gate would not
+// already do.
+//
+// Round 3's design ruling raised defaultBreakerMaxOpenDuration to 6h
+// (longer than the 1h interval) specifically so the breaker's backoff can
+// eventually become the binding constraint instead of the interval — see
+// that constant's own doc comment for the resulting cadence (roughly 5
+// attempts a day). This test now asserts the STRONGER, actually-intended
+// property: strictly fewer admits than the pre-feature baseline. Reverting
+// EITHER the round-2 interval-gate fix OR the round-3 default bump makes
+// this fail — the first by exceeding the baseline, the second by merely
+// equaling it.
+func TestProviderState_Breaker_DefaultCadence_StrictlyFewerThanPreFeatureBaseline(t *testing.T) {
 	t.Parallel()
 	st := newHealthTestState(defaultDiscoveryInterval, defaultBreakerFailureThreshold, defaultBreakerOpenDuration, defaultBreakerMaxOpenDuration)
 	base := time.Now()
@@ -275,8 +307,64 @@ func TestProviderState_Breaker_DefaultCadence_NoWorseThanPreFeatureBaseline(t *t
 	// all) admits exactly one attempt per defaultDiscoveryInterval (1h)
 	// over a 24h window for a provider that never succeeds.
 	const preFeatureBaseline = 24
-	if admits > preFeatureBaseline {
-		t.Errorf("admitted %d discovery attempts over 24h with default config and a permanently failing provider, want <= %d (the pre-feature, interval-only baseline) — the breaker must never retry MORE often than plain interval throttling", admits, preFeatureBaseline)
+	if admits >= preFeatureBaseline {
+		t.Errorf("admitted %d discovery attempts over 24h with default config and a permanently failing provider, want STRICTLY FEWER than %d (the pre-feature, interval-only baseline) — the breaker must actually suppress re-probing under stock config, not merely match plain interval throttling", admits, preFeatureBaseline)
+	}
+}
+
+// --- breaker: the admin-facing openUntil accounts for the interval floor
+// (MEDIUM adversarial-review finding, round 3) ---
+
+// TestProviderState_Snapshot_OpenUntil_ReflectsIntervalFloor proves
+// snapshot()'s emitted openUntil is the REAL next probe time —
+// max(raw openUntil, lastRefresh+interval) — not the raw backoff-only
+// value. Since tryBeginRefresh's interval gate applies unconditionally,
+// a fresh trip's raw openUntil (lastRefresh + breakerOpenBase) can be far
+// EARLIER than the actual next admitted attempt whenever breakerOpenBase
+// is shorter than interval — measured on pure defaults, a 59-minute skew
+// (raw +1m versus the real +1h). An operator watching the dashboard would
+// otherwise see "open (in 60s)", nothing happen, and then the countdown
+// silently vanish once formatUntil (webui) reads the now-past raw value.
+func TestProviderState_Snapshot_OpenUntil_ReflectsIntervalFloor(t *testing.T) {
+	t.Parallel()
+	// interval (1h) far longer than the backoff base (1m) — the exact
+	// shape of the shipped defaults on a fresh trip.
+	st := newHealthTestState(time.Hour, 1, time.Minute, 10*time.Minute)
+	base := time.Now()
+	st.tryBeginRefresh(base)
+	st.finishRefresh(base, nil, errors.New("boom")) // opens: raw openUntil = base+1m
+
+	_, lastRefresh, _, health, openUntil := st.snapshot()
+	if health != breakerOpen {
+		t.Fatalf("health = %v, want breakerOpen", health)
+	}
+	want := lastRefresh.Add(time.Hour)
+	if !openUntil.Equal(want) {
+		t.Errorf("snapshot openUntil = %v, want %v (lastRefresh + interval, not the raw +1m backoff — tryBeginRefresh will not actually admit a probe before then)", openUntil, want)
+	}
+}
+
+// TestProviderState_Snapshot_OpenUntil_UnchangedWhenBackoffExceedsInterval
+// is the complementary case: once backoff is the LONGER of the two (the
+// steady state this feature is designed to reach — see
+// defaultBreakerMaxOpenDuration's own doc comment), snapshot's openUntil
+// is exactly the raw value, with no floor adjustment needed.
+func TestProviderState_Snapshot_OpenUntil_UnchangedWhenBackoffExceedsInterval(t *testing.T) {
+	t.Parallel()
+	// interval (1m) far shorter than the backoff base (1h) — backoff is
+	// already the binding constraint from the very first trip.
+	st := newHealthTestState(time.Minute, 1, time.Hour, 10*time.Hour)
+	base := time.Now()
+	st.tryBeginRefresh(base)
+	st.finishRefresh(base, nil, errors.New("boom")) // opens: raw openUntil = base+1h
+
+	_, _, _, health, openUntil := st.snapshot()
+	if health != breakerOpen {
+		t.Fatalf("health = %v, want breakerOpen", health)
+	}
+	want := base.Add(time.Hour)
+	if !openUntil.Equal(want) {
+		t.Errorf("snapshot openUntil = %v, want %v (the raw backoff value, unchanged — it already exceeds the interval floor)", openUntil, want)
 	}
 }
 
@@ -422,13 +510,37 @@ func TestProviderState_Breaker_ContextCanceled_IsNeutral_NotSuccess(t *testing.T
 	}
 }
 
+// TestProviderState_Breaker_ContextCanceled_IsNeutral_NotFailure is the
+// other direction of the bidirectional check (round 3 finding): its
+// _NotSuccess sibling above proves Canceled does not reset the streak,
+// but says nothing about whether Canceled ITSELF is wrongly counted as a
+// failure — a mutant that classified Canceled as a failure would still
+// pass that test unchanged, since either way the threshold is reached by
+// the trailing plain failure. A LONE Canceled call on a threshold-1
+// breaker settles it directly: if Canceled counted as a failure, one call
+// would be enough to trip a threshold of 1.
+func TestProviderState_Breaker_ContextCanceled_IsNeutral_NotFailure(t *testing.T) {
+	t.Parallel()
+	st := newHealthTestState(time.Hour, 1, time.Minute, 10*time.Minute)
+	now := time.Now()
+
+	st.finishRefresh(now, nil, context.Canceled)
+
+	if !st.discoveryHealthy() {
+		t.Fatal("discoveryHealthy() = false after a single context.Canceled outcome on a threshold-1 breaker, want true — Canceled must not be counted as a failure")
+	}
+}
+
 // TestProviderState_Breaker_ContextCanceled_DuringHalfOpen_PreservesBackoff
 // is the sharper half of the same finding: an earlier version treated a
 // Canceled half-open probe as a SUCCESS, which CLOSED an already-open
 // breaker and wiped its backoff — reporting a still-401ing provider
 // healthy purely because a probe was interrupted at shutdown. This proves
-// the backoff survives: the NEXT real failure must escalate from the
-// ORIGINAL backoff, not restart fresh at the base.
+// two things: discoveryHealthy reports the provider unhealthy immediately
+// (round 3 finding: a canceled probe reverts to OPEN rather than being
+// left stuck at halfOpen, which would report healthy), and the backoff
+// survives — the NEXT real failure must escalate from the ORIGINAL
+// backoff, not restart fresh at the base.
 func TestProviderState_Breaker_ContextCanceled_DuringHalfOpen_PreservesBackoff(t *testing.T) {
 	t.Parallel()
 	st := newHealthTestState(time.Second, 1, time.Minute, 10*time.Minute)
@@ -442,6 +554,10 @@ func TestProviderState_Breaker_ContextCanceled_DuringHalfOpen_PreservesBackoff(t
 	}
 	st.finishRefresh(probeAt, nil, context.Canceled) // probe interrupted at shutdown: must be neutral
 
+	if st.discoveryHealthy() {
+		t.Fatal("discoveryHealthy() = true immediately after a canceled half-open probe, want false — a still-broken provider must not read healthy just because its re-probe was interrupted")
+	}
+
 	// The next real attempt must see the ORIGINAL backoff still in
 	// force, not a wiped/reset one: if Canceled had (incorrectly) closed
 	// the breaker, this next failure would re-open it FRESH at the 1m
@@ -499,6 +615,17 @@ func TestProviderState_Breaker_HealthyProviderNeverAffected(t *testing.T) {
 // nothing to catch a locking bug with; the locking was correct, but
 // unproven) ---
 
+// TestProviderState_Breaker_ConcurrentRefreshes_RaceFree is primarily a
+// -race and panic net (round 2), but round 3 found it carried ZERO
+// assertions and so survived every one of the seven fix-reversion
+// mutations tried against this file — useful for what it catches, but not
+// a behavioral test of its own. It now asserts two invariants on every
+// iteration, under the SAME lock discoveryHealthy/tryBeginRefresh already
+// take, so the read is race-free too: backoff never exceeds breakerOpenMax
+// (the overflow-ceiling property, round 2 finding 7, checked here under
+// real concurrent mutation rather than only sequentially), and health is
+// always one of the three declared states — breakerState is a bare int32
+// under the hood, so nothing enforces that at compile time.
 func TestProviderState_Breaker_ConcurrentRefreshes_RaceFree(t *testing.T) {
 	t.Parallel()
 	st := newHealthTestState(time.Millisecond, 3, 2*time.Millisecond, 20*time.Millisecond)
@@ -521,6 +648,18 @@ func TestProviderState_Breaker_ConcurrentRefreshes_RaceFree(t *testing.T) {
 					st.finishRefresh(now, []string{"m1"}, err)
 				}
 				_ = st.discoveryHealthy()
+
+				st.mu.Lock()
+				gotBackoff, gotMax, gotHealth := st.backoff, st.breakerOpenMax, st.health
+				st.mu.Unlock()
+				if gotBackoff > gotMax {
+					t.Errorf("worker %d iteration %d: backoff = %v, want <= breakerOpenMax %v", worker, i, gotBackoff, gotMax)
+				}
+				switch gotHealth {
+				case breakerClosed, breakerOpen, breakerHalfOpen:
+				default:
+					t.Errorf("worker %d iteration %d: health = %v, want one of closed/open/half-open", worker, i, gotHealth)
+				}
 			}
 		}(w)
 	}