`successThreshold` stops being read the moment recovery starts, so it can never require more than 2 consecutive successes

[os-warren]

Found while implementing #11852 (routing both health-check failure routes through the same
autoRestart handling). Outside that card's surface — it is fenced to restart eligibility and the
failure counters — so this is filed rather than fixed.

What was measured

packages/core/src/health-monitor.ts at f540920189, in performHealthCheck's success branch:

const currentStatus = this.healthStatus.get(pluginName);
if (currentStatus === 'unhealthy' || currentStatus === 'degraded') {
 const successCount = this.successCounters.get(pluginName) || 0;
 if (successCount >= config.successThreshold) {
 this.healthStatus.set(pluginName, 'healthy');
 } else {
 this.healthStatus.set(pluginName, 'recovering');
 }
} else {
 this.healthStatus.set(pluginName, 'healthy');
}

successThreshold is consulted only when the current status is unhealthy or degraded. The first
success moves the plugin to recovering — which is in neither set — so the second success takes
the else branch and goes straight to healthy without reading the counter at all. failed is in
neither set either, so a plugin that threw recovers on its first success.

So the declared value is capped in practice:

Status when the successes start	Consecutive successes actually required
unhealthy / degraded	2, whatever successThreshold says
failed / recovering	1, whatever successThreshold says

A declared successThreshold: 5 is indistinguishable from 2.

Declared contract

packages/spec/src/kernel/plugin-lifecycle-advanced.zod.ts:

successThreshold: z.number().int().min(1).default(1)
 .describe('Consecutive successes needed to mark healthy'),

content/docs/references/kernel/plugin-lifecycle-advanced.mdx:112 repeats it. Neither says the
count stops being read once recovery has begun, and content/docs/protocol/kernel/lifecycle.mdx
describes the failure side of the threshold machinery without describing this side.

The default is 1, which is exactly the value at which the defect is invisible — every declared
value above it is the one that misbehaves.

Why it is filed next to #11852

This is what makes the second asymmetry #11852 names — the catch path not clearing
successCounters — unobservable through the public API today. The read site above is the only
place successCounters is consulted, and it is unreachable with a stale non-zero counter: the
returned-failure route always zeroes the counter before unhealthy/degraded can be entered, and
the throw route sets failed, which the branch short-circuits past. #11852's fix performs the reset
(correct counter hygiene, and it becomes load-bearing the moment this card is addressed), but no
behavioural pin can distinguish it while the read site cannot be reached. That is recorded in that
PR rather than papered over with a test that would pass for the wrong reason.

What is NOT claimed

Which behaviour is intended is not determined here. "Consult successThreshold from every
non-healthy status" is the reading the describe() text suggests, but recovering may have been
meant as a terminal-ish display state rather than a counting one, and no ADR, comment or test states
an intent. health-monitor.test.ts asserts nothing about successThreshold on any route.

[claude]

Concentrated triage round: finding → pm:queue + domain:engine stands, Bug — declared≠enforced on a config key: successThreshold is consulted only while status ∈ {unhealthy, degraded}, and the first success moves status to recovering (in neither set), so the second success bypasses the counter entirely; the key can never demand more than 2. Premise re-verified on origin/main (health-monitor.ts:137). Hard serial: after #11852 (in flight, pm:dispatched, same health-check path in the same file) — dispatch only after its PR lands, on the merged ref.

---

Generated by Claude Code

[os-warren]

Claim: PM loop round 3
Session: session_01W6HFzyH98W1YaQXhJUJt6o
Branch: claude/issue-11955-success-threshold-read
Worktree: objectstack-issue-11955
Domain: domain:engine
File surface: packages/core/src/health-monitor.ts + packages/core/src/health-monitor.test.ts (stop on breach; explain in the report)
Container & model: M, mode:subagent, model: opus
Clause-②: no — successThreshold is a declared, documented config member (z.number().int().min(1).default(1), "Consecutive successes needed to mark healthy") that is enforced only from two of four statuses. Making it bind is declared ≠ enforced catching up on the enforcement side; no contract accept/reject door moves and no public surface widens.
Serial constraints cleared: none — health-monitor.ts is touched by no open PR (its last occupant, PR #11956, merged and its branch was auto-deleted). Disjoint from both cards in flight (schema-drift.ts #11535, sql-driver-aggregation-conformance.test.ts #11456) and from the open driver PRs.

Premise re-measured on origin/main — after the refactor that could have moved it

⚠️ This card was filed against PR #11956's head, and #11956 has since merged, refactoring performHealthCheck's failure routes. The obvious risk is that the refactor moved or fixed the success branch and left this card stale. It did not — measured, ⛔ not assumed:

origin/main:packages/core/src/health-monitor.ts
 :139 if (currentStatus === 'unhealthy' || currentStatus === 'degraded') {
 :141 if (successCount >= config.successThreshold) {
 :145 } else { … 'recovering' }
 :147 } else {
 :148 this.healthStatus.set(pluginName, 'healthy');

successThreshold is still read only inside the unhealthy || degraded gate. recovering and failed are in neither set, so they take the outer else and reach healthy without the counter being consulted at all. #11956's change is visible one line below (failureRoute = 'returned') and is confined to the failure routes.

⇒ The defect is exactly as filed. A declared successThreshold: 5 is still indistinguishable from 2 — and from 1 when recovery starts at failed.

Why this card matters more than its size

⭐ It is the reason #11852 shipped a correct fix with no behavioural pin. That card fixed the catch path's missing successCounters reset, and could not test it: the counter's only read site is unreachable with a stale non-zero value, so any test would have passed for the wrong reason. The dev reported that honestly instead of writing the reassuring test.

⇒ Closing this makes that reset observable, and load-bearing. ⚠️ Whoever takes it should read #11852's landed diff first — the two interlock, and a fix here that ignores the reset would re-open the asymmetry from the other side.

⚠️ The one thing this card does NOT decide, and must not decide silently

The card is explicit that intent is undetermined:

"Consult successThreshold from every non-healthy status" is the reading the describe() text suggests, but recovering may have been meant as a terminal-ish display state rather than a counting one, and no ADR, comment or test states an intent.

⇒ Two defensible shapes, and they differ in observable behaviour:

• (a) count from every non-healthy status (recovering and failed included) — the reading the declared text supports;
• (b) keep recovering non-counting and fix only the arithmetic.

Pick the one the declaration supports, state which you picked and why in the PR body, and pin the chosen semantics. ⚠️ If measurement shows the two are not distinguishable by any declared text — i.e. this is a genuine product question rather than a defect — that is a fork: stop and report, and it becomes a decision card. ⛔ Do not pick a side quietly because one is easier to test.

⚠️ Note the default is 1, exactly the value at which the defect is invisible. Every declared value above it is the one that misbehaves — so a pin at the default proves nothing.

---

Generated by Claude Code