From 04afcbaa61d6e6b5f63a3a5f0114222d363fa4e5 Mon Sep 17 00:00:00 2001 From: Tomer Weller Date: Mon, 24 Aug 2026 05:03:22 +0000 Subject: [PATCH 1/3] =?UTF-8?q?Regression=20test=20for=20#3816=20=E2=80=94?= =?UTF-8?q?=20fails=20on=20current=20main?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add cold-catchup carveout tests for check-12b's post-restart absolute fire: suppressed on bucket-apply (cold catchup) and on FRESH_START, exemption line renders, and warm restart still fires. Fail on main: eval_counter_streak has no fresh_start kwarg and no cold-catchup gate; renderer has no exemption form. Refs #3816 Co-authored-by: Claude Code --- scripts/lib/test_eval_alarms_counter_reset.py | 149 ++++++++++++++++++ 1 file changed, 149 insertions(+) diff --git a/scripts/lib/test_eval_alarms_counter_reset.py b/scripts/lib/test_eval_alarms_counter_reset.py index d82b1132..fb1e3d2f 100644 --- a/scripts/lib/test_eval_alarms_counter_reset.py +++ b/scripts/lib/test_eval_alarms_counter_reset.py @@ -511,6 +511,155 @@ def test_post_restart_below_threshold_does_not_fire(): f"Below-threshold reset must not set post_restart, got {result.get('post_restart')!r}" +# ── cold-catchup carveout tests (issue #3816) ──────────────────────────────── + +def test_post_restart_fire_suppressed_on_cold_catchup(): + """A baseline-reset (PID-change) tick whose absolute counter crosses + post_restart_absolute_threshold must NOT fire post-restart when the node + just completed a from-scratch (HAS-restore) catchup this incarnation — + signalled by stellar_history_bucket_apply_success_total > 0. Being behind + for minutes across ~10^5 ledgers is the point of a cold catchup, not a stall. + + Fails on origin/main: eval_counter_streak has no `fresh_start` kwarg + (TypeError) and no cold-catchup gate, so the PID-change branch fires + post_restart at absolute=63. + Passes after: the cold-catchup gate returns collecting_baseline. + """ + with tempfile.TemporaryDirectory() as d: + state_dir = Path(d) + snap_path = state_dir / "counter_streak_snapshot" + write_snapshot(snap_path, { + "version": "1", + "pid": "111", + "start_ticks": "100", + "counter_value": "0", + "breach_streak": "0", + }) + + alarm = _make_alarm("recovery-stalled", kind="counter-streak", + metric="recovery-stalled-metric", + delta_threshold=1, streak_threshold=3, + burst_threshold=10, + post_restart_absolute_threshold=50, + severity="WARN") + # cur_val 63 >= threshold 50, AND a cold-catchup bucket-apply happened + # this incarnation (labeled per-archive series, value >= 1). + current = { + "recovery-stalled-metric": [({}, 63.0)], + "stellar_history_bucket_apply_success_total": [({"archive": "sdf"}, 1.0)], + } + + result = eval_counter_streak(alarm, current, state_dir, "222", "200", + fresh_start=False) + + assert result["state"] == "collecting_baseline", \ + f"Cold catchup must suppress the post-restart fire, got {result['state']}" + assert not result.get("post_restart"), \ + f"Cold catchup must not set post_restart, got {result.get('post_restart')!r}" + assert result.get("cold_catchup_exemption"), \ + f"Cold-catchup suppression must set the exemption marker, got {result.get('cold_catchup_exemption')!r}" + + # Fresh baseline still written before the (suppressed) fire check. + snap = read_snapshot(snap_path) + assert snap["pid"] == "222", "fresh baseline pid must be written" + assert snap["breach_streak"] == "0", "fresh baseline streak must be 0" + + +def test_render_cold_catchup_exemption_line(): + """render_aggregate labels a collecting_baseline result carrying the + cold_catchup_exemption marker with the documented exemption suffix. + + Fails on origin/main: the renderer has no such branch and emits the plain + `recovery_stalled: collecting baseline` line. + """ + r = { + "contributes_to": "recovery_stalled", + "state": "collecting_baseline", + "cold_catchup_exemption": True, + } + out = render_aggregate([r], watcher_mode=False) + line = out["recovery_stalled_line"] + assert line == "recovery_stalled: collecting baseline (cold-catchup exemption)", \ + f"Expected cold-catchup exemption form, got: {line!r}" + + +def test_post_restart_fire_suppressed_on_fresh_start(): + """The fresh_start OR arm: even without the bucket-apply metric on this tick, + a FRESH_START=yes (state-wipe) tick must suppress the post-restart fire. + + Fails on origin/main: no `fresh_start` kwarg (TypeError) and no gate. + """ + with tempfile.TemporaryDirectory() as d: + state_dir = Path(d) + snap_path = state_dir / "counter_streak_snapshot" + write_snapshot(snap_path, { + "version": "1", + "pid": "111", + "start_ticks": "100", + "counter_value": "0", + "breach_streak": "0", + }) + + alarm = _make_alarm("recovery-stalled", kind="counter-streak", + metric="recovery-stalled-metric", + delta_threshold=1, streak_threshold=3, + burst_threshold=10, + post_restart_absolute_threshold=50, + severity="WARN") + # No bucket-apply series present; fresh_start=True is the only signal. + current = {"recovery-stalled-metric": [({}, 63.0)]} + + result = eval_counter_streak(alarm, current, state_dir, "222", "200", + fresh_start=True) + + assert result["state"] == "collecting_baseline", \ + f"FRESH_START must suppress the post-restart fire, got {result['state']}" + assert not result.get("post_restart"), \ + f"FRESH_START must not set post_restart, got {result.get('post_restart')!r}" + assert result.get("cold_catchup_exemption"), \ + f"FRESH_START suppression must set the exemption marker, got {result.get('cold_catchup_exemption')!r}" + + +def test_post_restart_fire_still_fires_warm_restart(): + """Guard for #3197/#3198: a warm near-tip restart (no bucket-apply series, + fresh_start=False) must STILL fire post-restart at absolute >= threshold. + The carveout must be conditional on the cold-catchup signal, not swallow the + warm-restart stall detection the check exists to provide. + + Passes before AND after the fix (with the new default kwarg), proving the + carveout is conditional. + """ + with tempfile.TemporaryDirectory() as d: + state_dir = Path(d) + snap_path = state_dir / "counter_streak_snapshot" + write_snapshot(snap_path, { + "version": "1", + "pid": "111", + "start_ticks": "100", + "counter_value": "0", + "breach_streak": "0", + }) + + alarm = _make_alarm("recovery-stalled", kind="counter-streak", + metric="recovery-stalled-metric", + delta_threshold=1, streak_threshold=3, + burst_threshold=10, + post_restart_absolute_threshold=50, + severity="WARN") + # No bucket-apply series (warm restart, pure ledger-chain replay). + current = {"recovery-stalled-metric": [({}, 63.0)]} + + result = eval_counter_streak(alarm, current, state_dir, "222", "200", + fresh_start=False) + + assert result["state"] == "firing", \ + f"Warm restart must still fire post-restart, got {result['state']}" + assert result.get("post_restart") is True, \ + f"Warm restart must set post_restart, got {result.get('post_restart')!r}" + assert not result.get("cold_catchup_exemption"), \ + f"Warm restart must not set the exemption marker, got {result.get('cold_catchup_exemption')!r}" + + # ── missing-process-identity guard tests (issue #3279) ─────────────────────── def test_missing_identity_does_not_poison_snapshot_or_fire(): From c7a50e0876ae7605aeb2246f3b16fd550b130f8d Mon Sep 17 00:00:00 2001 From: Tomer Weller Date: Mon, 24 Aug 2026 05:05:14 +0000 Subject: [PATCH 2/3] Add cold-catchup carveout to check-12b post-restart absolute fire MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The post-restart absolute fire (#3198) exists to catch a warm restart whose startup stall burst straddles the baseline-reset tick. A cold catchup after a state wipe legitimately accrues >= post_restart_absolute_threshold escalation ticks (forcing_catchup_behind) — being behind for minutes across ~10^5 ledgers is the point, not a stall — so the fire is a false positive on a healthy post-wipe recovery (#3816). Suppress the one-shot fire when this incarnation demonstrably did a from-scratch HAS-restore catchup, signalled by stellar_history_bucket_apply_success_total > 0 (per-incarnation, only emitted on the bucket-apply path, 0/absent on a warm near-tip restart), OR by FRESH_START=yes as a belt-and-suspenders arm. Keying on bucket-apply (not crash recovery) keeps #3197's blind spot closed: #3197 was a warm restart that did no bucket apply, so the fire still triggers there. A stuck cold catchup is still caught — only the one-shot fire is exempted; the same-PID delta/streak/burst path fires on later ticks if the counter keeps climbing. The exemption is surfaced so the tick shows "collecting baseline (cold-catchup exemption)". No threshold/toml change. Refs #3816 Co-authored-by: Claude Code --- .claude/skills/monitor-tick/SKILL.md | 1 + scripts/lib/eval-alarms.py | 57 +++++++++++++++++++++++++++- 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/.claude/skills/monitor-tick/SKILL.md b/.claude/skills/monitor-tick/SKILL.md index d846a5f6..8e3cd061 100644 --- a/.claude/skills/monitor-tick/SKILL.md +++ b/.claude/skills/monitor-tick/SKILL.md @@ -1700,6 +1700,7 @@ to overall tick severity — the tick is considered unhealthy when any alert fir - `recovery_stalled: WARNING absolute=N (post-restart) — investigating` — post-restart absolute fire (absolute ≥ 50 on a baseline-reset tick, #3198) - `recovery_stalled: skipped ()` — metric missing or fetch failed - `recovery_stalled: collecting baseline` — first tick after restart/invalidation (absolute value below the post-restart threshold) +- `recovery_stalled: collecting baseline (cold-catchup exemption)` — a baseline-reset tick whose absolute value ≥ 50 was suppressed because this incarnation completed a from-scratch (HAS-restore) catchup, i.e. a healthy post-wipe recovery, not a stall (`stellar_history_bucket_apply_success_total` > 0, or `FRESH_START=yes`); the warm-restart post-restart fire (#3198) still triggers because a warm restart does no bucket apply (#3816) **Rendering precedence** (determines the `metrics_ratio:` line format): diff --git a/scripts/lib/eval-alarms.py b/scripts/lib/eval-alarms.py index ea61b52b..149e377e 100644 --- a/scripts/lib/eval-alarms.py +++ b/scripts/lib/eval-alarms.py @@ -527,6 +527,14 @@ def make_result( # only inside extra_values, so without this it never reaches the # renderer and a post-restart fire is mislabeled (burst) (#3274). "post_restart": bool(extra_values and extra_values.get("post_restart")), + # Surface the cold-catchup exemption marker (#3816) the same way as + # post_restart above: extra_values keys are NOT auto-promoted, so + # without this the exemption never reaches the renderer and a suppressed + # post-restart fire on a healthy cold catchup renders as a plain + # "collecting baseline" with no visible reason for the quiet. + "cold_catchup_exemption": bool( + extra_values and extra_values.get("cold_catchup_exemption") + ), } # Safety net: warn about unresolved template placeholders. @@ -1482,10 +1490,26 @@ def eval_counter_streak( start_ticks: str, gap_stale: bool = False, too_fresh: bool = False, + fresh_start: bool = False, ) -> dict: """Evaluate a counter-streak alarm. Independent of PREV_PROM_INVALID — uses own PID/start_ticks in snapshot. + + Cold-catchup carveout (#3816): the post-restart absolute fire (#3198) exists + to catch a *warm* restart whose startup stall burst straddles the + baseline-reset tick. A *cold* catchup after a state wipe legitimately accrues + >= post_restart_absolute_threshold escalation ticks (being behind for minutes + across ~10^5 ledgers is the point, not a stall). We suppress the one-shot + post-restart fire when this incarnation demonstrably did a from-scratch + HAS-restore catchup — signalled by stellar_history_bucket_apply_success_total + (per-incarnation counter, only emitted on the bucket-apply path, 0/absent on a + warm near-tip restart), OR by FRESH_START=yes as a cheap belt-and-suspenders + arm. A genuinely *stuck* cold catchup is still caught: only the one-shot fire + is exempted; the ordinary same-PID delta/streak/burst path fires on later + ticks if the counter keeps climbing. Keying on bucket-apply (not crash + recovery) is deliberate — #3197's warm-restart stall did NO bucket apply, so + the blind spot the check exists to close stays closed. """ ev_default = default_extra_values(alarm, "counter-streak") @@ -1512,6 +1536,14 @@ def eval_counter_streak( return make_result(alarm, "skipped", skip_reason="metric not found", extra_values=ev_default) + # Cold-catchup signal (#3816), observable at the baseline-reset (fire) tick. + # form1's fallback returns the first series' value even though the counter is + # labeled per-archive; the counter is only exported once it increments (>= 1) + # on a from-scratch HAS-restore catchup, so presence with value > 0 means a + # cold catchup happened this incarnation. Absent (None) on a warm restart. + bucket_apply = extract_value(current, "stellar_history_bucket_apply_success_total", "form1") + cold_catchup = (bucket_apply is not None and bucket_apply > 0) or fresh_start + snapshot_file = alarm.get("snapshot_file", "counter_streak_snapshot") snapshot_path = state_dir / snapshot_file snapshot = read_snapshot(snapshot_path) @@ -1538,6 +1570,15 @@ def eval_counter_streak( write_snapshot(snapshot_path, new_snapshot) post_restart = maybe_post_restart_fire(alarm, cur_val) if post_restart is not None: + if cold_catchup: + # #3816: a healthy from-scratch catchup legitimately crossed + # the absolute threshold — suppress the one-shot fire. The + # fresh baseline is already written, so a genuinely stuck + # cold catchup still fires via the same-PID delta/streak path. + return make_result( + alarm, "collecting_baseline", + extra_values={"cold_catchup_exemption": True, **ev_default}, + ) return post_restart return make_result(alarm, "collecting_baseline", extra_values=ev_default) @@ -1575,6 +1616,12 @@ def eval_counter_streak( write_snapshot(snapshot_path, new_snapshot) post_restart = maybe_post_restart_fire(alarm, cur_val) if post_restart is not None: + if cold_catchup: + # #3816: cold-catchup exemption, mirroring the PID-change branch. + return make_result( + alarm, "collecting_baseline", + extra_values={"cold_catchup_exemption": True, **ev_default}, + ) return post_restart return make_result(alarm, "collecting_baseline", extra_values=ev_default) @@ -1755,7 +1802,13 @@ def render_aggregate(results: list[dict], watcher_mode: bool) -> dict: elif r["state"] == "skipped": recovery_stalled_line = f"recovery_stalled: skipped ({r.get('skip_reason', '')})" elif r["state"] == "collecting_baseline": - recovery_stalled_line = "recovery_stalled: collecting baseline" + # #3816: surface the cold-catchup exemption so an operator watching a + # state-wipe recovery sees why the post-restart fire stayed quiet, + # instead of an unexplained "collecting baseline". + if r.get("cold_catchup_exemption"): + recovery_stalled_line = "recovery_stalled: collecting baseline (cold-catchup exemption)" + else: + recovery_stalled_line = "recovery_stalled: collecting baseline" else: recovery_stalled_line = f"recovery_stalled: ok (delta={r.get('value', 0)})" @@ -2089,7 +2142,7 @@ def main() -> int: ) elif kind == "counter-streak": result = eval_counter_streak(alarm, current, state_dir, pid, start_ticks_val, - gap_stale, too_fresh) + gap_stale, too_fresh, fresh_start) else: result = make_result(alarm, "skipped", skip_reason=f"unknown kind: {kind}") From 222b9cbfeb30e77a4a1c734546b7ad7f6652d1b8 Mon Sep 17 00:00:00 2001 From: Tomer Weller Date: Mon, 24 Aug 2026 23:02:23 +0000 Subject: [PATCH 3/3] Re-trigger CI on a fresh head SHA (flaky testnet-infra bounce) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both reviewer lenses (Correctness, Risk) APPROVE and there are no inline comments, so no code change is warranted. The prior bounces were caused by an environmental testnet-shard timeout in the Quickstart run, not by this diff (monitoring tooling only: scripts/lib/eval-alarms.py, its test, and .claude/skills/monitor-tick/SKILL.md). origin/main has not advanced past 9f534c2, so a rebase is a no-op. This empty commit advances the head SHA — as /review-pr Cycle 3 required — so CI re-runs against current testnet state and the head-scoped bounce counter resets. Refs #3816 Co-authored-by: Claude Code