Add cold-catchup carveout to check-12b post-restart absolute fire

[tomerweller]

Closes #3816

Summary

check-12b's post-restart absolute fire (#3198) WARNs on a baseline-reset tick whenever the discarded forcing_catchup_behind counter is ≥ 50, to catch a warm restart whose startup stall burst straddles the reset tick. A cold catchup after a state wipe legitimately accrues ≥ 50 escalation ticks (63 across ~137k ledgers in the reported case) — being behind for minutes is the whole point of a from-scratch catchup, not a stall — so the fire is a false positive on a healthy post-wipe recovery.

This adds a cold-catchup carveout in eval_counter_streak: the one-shot post-restart fire is suppressed when this incarnation demonstrably did a from-scratch HAS-restore catchup, signalled by stellar_history_bucket_apply_success_total > 0 (a 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 belt-and-suspenders arm. Keying on bucket-apply — not crash recovery — keeps the #3197/#3198 blind spot closed: #3197 was a warm restart that did no bucket apply, so the fire still triggers there. A genuinely stuck cold catchup is still caught, since only the one-shot fire is exempted and the same-PID delta/streak/burst path fires on later ticks if the counter keeps climbing. The exemption is surfaced so the tick renders recovery_stalled: collecting baseline (cold-catchup exemption). No threshold or toml change.

Plan reference

Converged Plan comment

Test plan

• python3 scripts/lib/test_eval_alarms_counter_reset.py — 32 passed
• full scripts/lib/test_eval_alarms_*.py suite — all green
• python3 -m py_compile scripts/lib/eval-alarms.py
• n/a: no Rust changed (monitoring tooling only), so cargo fmt/clippy are not exercised by this diff

Regression test (kind: bug-fix)

• Test: scripts/lib/test_eval_alarms_counter_reset.py::test_post_restart_fire_suppressed_on_cold_catchup (+ test_render_cold_catchup_exemption_line, test_post_restart_fire_suppressed_on_fresh_start, test_post_restart_fire_still_fires_warm_restart)
• Pre-fix: committed as 04afcba — verified FAILED with: eval_counter_streak() got an unexpected keyword argument 'fresh_start' and renderer emitting the plain collecting baseline line
• Post-fix: verified PASSES after c7a50e0

Deviations from plan

• The plan scoped the doc touch to .claude/skills/monitor-tick/SKILL.md. There is also a .agents/skills/monitor-tick/SKILL.md mirror, but the two already diverge at HEAD (not kept byte-identical), so I updated only the .claude copy per plan scope rather than reconciling a stale mirror.

🤖 Generated with Claude Code

[tomerweller]

🔍 Reviewer: Correctness

Verdict: APPROVE

Summary: Focused, well-tested cold-catchup carveout in eval_counter_streak. The one-shot post-restart absolute fire (#3198) is suppressed only when a from-scratch HAS-restore catchup is demonstrable (stellar_history_bucket_apply_success_total > 0 or FRESH_START=yes); the warm-restart blind spot (#3197/#3198) stays covered and a genuinely stuck cold catchup still fires via the same-PID delta/streak path.

Full review

Cycle 1 — complete class-labeled assessment (no prior Correctness comment on this PR).

• correctness: The gate is applied in both baseline-reset branches (PID change and start_ticks change), each after the fresh baseline is written, so a still-stuck cold catchup re-fires on later ticks via the ordinary delta/streak/burst path. The cold_catchup signal (bucket_apply is not None and bucket_apply > 0) or fresh_start correctly captures the "did a bucket apply this incarnation" semantics; extract_value(..., "form1") returning the first per-archive series is acceptable for a boolean > 0 gate.
• correctness (extra_values promotion): make_result now promotes cold_catchup_exemption the same way post_restart is promoted (extra_values keys are not auto-promoted). Verified default_extra_values(alarm, "counter-streak") returns only {"streak", "streak_threshold"}, so the {"cold_catchup_exemption": True, **ev_default} spread ordering cannot be clobbered by ev_default.
• correctness (renderer): render_aggregate emits the documented collecting baseline (cold-catchup exemption) suffix only when the marker is set; default path unchanged.
• api-shape (wiring): main() now threads fresh_start into the counter-streak call; the new kwarg is defaulted (fresh_start: bool = False) so no other caller breaks.
• test-coverage (bug-fix gate): Verified eval_counter_streak on origin/main has no fresh_start kwarg and no cold-catchup gate — the regression test test_post_restart_fire_suppressed_on_cold_catchup raises TypeError there, i.e. provably fails pre-fix. New tests cover: cold-catchup suppression, fresh_start OR-arm, warm-restart still-fires guard, and the renderer line. Full test_eval_alarms_counter_reset.py suite: 28 passed, 0 failed locally; py_compile clean.
• doc-drift: monitor-tick/SKILL.md gains a precise entry for the new state line. PR notes the .agents/ mirror already diverges at HEAD, so scoping to the .claude copy is reasonable.

No blocking concerns.

[tomerweller]

🔍 Reviewer: Risk

Verdict: APPROVE

Summary: Monitoring-tooling-only change (no crates/ touched, so non-parity → risk lens). No regression, security, perf, or operational risk. The carveout narrows exactly one false-positive path and is additive/backward-compatible; the alerting blind spot the check exists to close (#3197/#3198 warm restart) is explicitly preserved and covered by a still-fires guard test.

Full review

Cycle 1 — complete class-labeled assessment (no prior Risk comment on this PR).

• regression-risk: New kwarg fresh_start: bool = False is defaulted; the only production caller (main()) is updated to pass it. No behavior change on the warm-restart path — test_post_restart_fire_still_fires_warm_restart asserts a warm restart at absolute >= threshold still fires and does not set the exemption marker. The suppression is strictly conditional on a demonstrable cold-catchup signal.
• operational (alerting coverage): The change reduces a false-positive WARN on healthy post-wipe recovery without weakening detection of a genuinely stuck cold catchup — only the one-shot post-restart fire is exempted; the same-PID delta/streak/burst path still fires on subsequent ticks. Exemption is surfaced to the operator via the new collecting baseline (cold-catchup exemption) line, so quiet ticks are explained rather than silent.
• security: None — no new inputs, no external surface, pure metric-value gating.
• perf: Negligible — one extra extract_value lookup per counter-streak eval.
• data-format / api: No wire/XDR/RPC/CLI contract touched. Not parity-relevant per docs/PARITY.md (monitoring tooling only).
• ci-failure (noted, not a code concern): The Quickstart run is marked cancelled solely because one test (testnet, core,horizon, ...) job stalled on its live-network "Run probes through wrapper" step for ~55 min and was cancelled. This is unrelated to the diff (Python monitoring tooling + skill markdown only) — flaky testnet infra, not diff-attributable.

No blocking concerns.

[tomerweller]

Do: Re-triggered CI (Mode B — CI-failure bounce)

Both reviewer lenses APPROVE and no inline changes were requested, so no code change was warranted. The bounce reason was a flaky testnet-infra cancellation of the Quickstart run, not a diff-attributable failure.

• Rebased do/issue-3816 on origin/main: no-op — main has not advanced past 9f534c2, so the branch head is unchanged at c7a50e0 (no new commit, no force-push).
• Local re-verification on the head commit: py_compile scripts/lib/eval-alarms.py clean; test_eval_alarms_counter_reset.py — 32 passed, 0 failed.
• Re-ran the cancelled Quickstart workflow (--failed, run 32692312669) to clear the stalled testnet-probe job. Now in progress.

Ready for re-review once CI settles.

[tomerweller]

Do: Re-triggered CI (Mode B — CI-failure bounce, 2nd occurrence)

Both reviewer lenses (Correctness, Risk) APPROVE and there are no inline comments, so no code change is warranted. The bounce reason is again an environmental testnet-shard failure, not a diff-attributable one.

• Failure is environmental, not the diff. The only red check is test (testnet, core,horizon, ...). Its log shows the testnet sync probes timing out — Transient-infra failure (exit 124/137) on retryable shard ... retrying, then SOFT-SKIP: testnet sync probe timed out (environmental, not a henyey failure), and finally The action 'Run probes through wrapper' has timed out after 25 minutes. This is the external testnet quickstart harness (CI: Quickstart testnet shard cancelled on ≥2 consecutive main commits — probe wrapper hangs at step 8 (systematic, not flaky) #3272 soft-on-timeout shard), not henyey.
• Scope confirms non-attributable. The PR touches only monitoring tooling — scripts/lib/eval-alarms.py, scripts/lib/test_eval_alarms_counter_reset.py, .claude/skills/monitor-tick/SKILL.md. No crates/ code, so the testnet node behaviour is unchanged from main.
• Local re-verification on the head commit (c7a50e08). py_compile scripts/lib/eval-alarms.py clean; test_eval_alarms_counter_reset.py — 32 passed, 0 failed.
• Rebase on origin/main: no-op. merge-base(main, c7a50e08) == origin/main (9f534c2), so the branch is already current; no force-push.
• Re-ran the failed jobs on run 32692312669 to clear the stalled testnet probe.

Ready for re-review once CI settles. If the testnet shard times out again environmentally, this is a candidate for /review-pr force-converge given both lenses approve and the failure is off-diff.

[tomerweller]

Review: Bounce-Back Cycle 3

Reason: CI failed (unrelated, will rebase)

Reviewer A: APPROVE — cold-catchup carveout is correct and well-scoped; bug-fix regression test provably fails on origin/main. Unchanged since Cycle 1 (same head c7a50e0876).
Reviewer B: APPROVE — monitoring-tooling-only (non-parity); no regression/security/perf/operational risk; warm-restart detection preserved and guard-tested. Unchanged since Cycle 1.
External reviewers: none
CI: red

Both reviewer lenses APPROVE. Every code/build/test/lint check is green. The one red signal is a single Quickstart job — test (testnet, core,horizon, test_core.go test_horizon_up.go ...) (run 32692312669, job 97607932322) — which failed on its live-testnet probe step. Its earlier steps (checkout, image load, container run) succeeded; only the live-network testnet probe failed.

This is unrelated to the diff: this PR changes only scripts/lib/eval-alarms.py, its test file, and .claude/skills/monitor-tick/SKILL.md — none of which touch the quickstart / core / horizon / testnet-probe path. It is a flaky testnet-infra failure, not diff-attributable.

Critical — head has NOT advanced. The head commit is still c7a50e0876 (2026-08-24T05:05:14Z), unchanged across Cycles 1 and 2. The prior two bounces both requested a genuine rebase on origin/main (advancing the head SHA) to re-trigger CI against current testnet state; that rebase never happened. /do Mode B MUST advance the head SHA this cycle — a bare CI re-trigger on the same tree will not clear a flaky-infra red and will not reset the head-scoped bounce counter.

Last bounce before the head-scoped cap. This is bounce 3 on the current code state. If the head is still not advanced on the next /review-pr tick, the head-scoped cap (>=3) engages and this issue will be moved to blocked for operator attention (re-run the flaky testnet job, rebase manually, or merge manually given both reviewers APPROVE).

Routing back to ready-for-doing.

[tomerweller]

Do: Re-triggered CI on a fresh head SHA (Mode B — CI-failure bounce, cycle 4)

Both reviewer lenses (Correctness, Risk) APPROVE and there are no inline comments, so no code change is warranted. The bounce reason is again the environmental testnet-shard timeout in the Quickstart run (test (testnet, core,horizon, ...)), not diff-attributable — this PR touches only scripts/lib/eval-alarms.py, its test, and .claude/skills/monitor-tick/SKILL.md.

Per the Cycle 3 requirement to advance the head SHA (a bare CI re-trigger on the same tree does not reset the head-scoped bounce counter):

• Head advanced c7a50e08 → 222b9cb. origin/main has not moved past 9f534c2, so a rebase is a genuine no-op; the head is advanced via an empty commit that re-runs CI against current testnet state.
• Local re-verification on the new head: py_compile scripts/lib/eval-alarms.py clean; test_eval_alarms_counter_reset.py — 32 passed, 0 failed.

Ready for re-review. The head-scoped bounce counter is now reset to the new SHA.

[tomerweller]

🔍 Reviewer: Correctness

Verdict: APPROVE

Summary: Cold-catchup carveout is correct, well-scoped, and covered by a bug-fix regression test that provably fails at the pre-fix commit and passes after the fix.

Full review

Reviewed the PR head (222b9cb) merged with current origin/main (9f534c2 — merge is a no-op; PR is current).

• Logic (eval_counter_streak): cold_catchup = (bucket_apply is not None and bucket_apply > 0) or fresh_start is computed once and gates the one-shot post-restart fire in both the PID-change and start_ticks-change branches. In each branch the fresh baseline snapshot is written before the exemption returns, so a genuinely stuck cold catchup is still caught on later ticks via the ordinary same-PID delta/streak/burst path. Correct.
• make_result promotion: cold_catchup_exemption is promoted from extra_values to a top-level key, mirroring the existing post_restart promotion (Non-critical: monitor-tick: post-restart absolute recovery_stalled fire mislabeled as (burst) delta #3274). Without it the marker never reaches the renderer. Correct pattern.
• Renderer: the exemption suffix is only emitted for collecting_baseline results carrying the marker; the plain line is preserved otherwise.
• main() wiring: fresh_start is threaded through to eval_counter_streak. Verified.
• Test coverage (kind: bug-fix): Verified the 4 new tests FAIL at pre-fix commit 04afcba (eval_counter_streak() got an unexpected keyword argument 'fresh_start'; renderer emits plain collecting baseline) and PASS on the fix. test_post_restart_fire_still_fires_warm_restart guards the URGENT: post-restart consensus stall recurs on #3179 binary (519665ad) — ~6.5min outage, recurrence of #3181/#1862 #3197/Non-critical: monitor-tick: post-restart forcing_catchup_behind stall invisible to checks (2) and (12b) #3198 blind spot — a warm restart (no bucket-apply) still fires. Full test_eval_alarms_* suite green (32 in counter_reset + all sibling suites); py_compile clean.

No blocking concerns.

[tomerweller]

🔍 Reviewer: Risk

Verdict: APPROVE

Summary: Monitoring-tooling-only change (Python + skill doc); no observable/interop-parity surface, no regression/security/perf/operational risk. Warm-restart stall detection is preserved and guard-tested.

Full review

Changed files: scripts/lib/eval-alarms.py, scripts/lib/test_eval_alarms_counter_reset.py, .claude/skills/monitor-tick/SKILL.md. Non-parity (no crates/{scp,herder,ledger,tx,overlay} paths; no Rust, no XDR, no wire/hash surface). No threshold or toml change.

• Regression risk: the carveout is strictly conditional on the cold-catchup signal (stellar_history_bucket_apply_success_total > 0 OR FRESH_START=yes). A warm near-tip restart does no bucket apply, so the Non-critical: monitor-tick: post-restart forcing_catchup_behind stall invisible to checks (2) and (12b) #3198 post-restart fire still triggers there — the URGENT: post-restart consensus stall recurs on #3179 binary (519665ad) — ~6.5min outage, recurrence of #3181/#1862 #3197 blind spot the check exists to close stays closed. Guard-tested by test_post_restart_fire_still_fires_warm_restart.
• Missed-stall risk: only the one-shot post-restart fire is exempted. The fresh baseline is written before the exemption, so a genuinely stuck cold catchup still fires via the same-PID delta/streak/burst path on later ticks.
• Operational: the exemption is surfaced (recovery_stalled: collecting baseline (cold-catchup exemption)) so an operator watching a state-wipe recovery sees why the fire stayed quiet, rather than an unexplained quiet.

No blocking concerns.

[tomerweller]

Review: Blocked — Unrelated CI Red Persists After Head Advance

Status: blocked
Reviewer A (Correctness): APPROVE
Reviewer B (Risk): APPROVE
External reviewers: none
CI: red — but not diff-attributable
Head-scoped bounce count: 0 (fresh SHA) · Lifetime bounce count: 2

Pattern: For three cycles now the only red signal has been a single Quickstart job — test (testnet, core,horizon, ...) — whose live-network "Run probes through wrapper" step hangs and is cancelled (Cycle 1: ~55 min stall→cancel; Cycle 2: testnet-shard probe failure; now: cancelled again mid-probe on the fresh head 222b9cb). Every other check — all of CI (build, Test, Clippy, Doctest, CodeQL, fmt, contract, script self-tests, skill snippets), plus the local and pubnet Quickstart shards — is green.

This is environmental testnet-infra flakiness, not diff-attributable: the PR changes only scripts/lib/eval-alarms.py, its test, and .claude/skills/monitor-tick/SKILL.md, none of which touch the quickstart / core / horizon / testnet-probe path.

Per the Cycle 2 escalation plan ("if the testnet job fails again after a genuine rebase / head-SHA advance and remains red, the next /review-pr should mark this blocked rather than bouncing a third time"), and because origin/main has not moved past 9f534c2 (a rebase is a genuine no-op, so /do has no further autonomous remediation), this is now routed to blocked for operator attention.

Both agent reviewer lenses APPROVE — the code is correct and well-scoped; the block is purely the persistent, unrelated testnet-shard cancellation. Operator options:

• Merge manually with --admin (the red check is unrelated to the diff), or
• Re-run just the flaky testnet Quickstart shard until it passes, then re-pick, or
• Adjust branch protection so the flaky live-testnet shard isn't merge-gating.

To retry via the pipeline after the cause clears, post ## Review: Reset with a one-line reason.