[sonichi]

The bug

check_session_cron_registration counts every entry with a cron string toward expected. That includes an entry parked on a date that can never occur.

This host's crons.json carries wire-newsroom-nightly-DISABLED-2026-06-09-1425Z-... at 0 0 31 2 * — February 31st. /schedule-crons registers the other 8 and cannot register that one, so the stamp reads 8/9 and the guard warns on every single run.

It cannot clear on its own. The only exits are re-enabling the parked job (the owner's call alone) or deleting the disabled record along with the reason embedded in its name.

Why this is worth fixing rather than tolerating

The guard exists to catch a silent failure — a peer instance registered 2/18 crons with no error, config intact on disk, zero live crons. A probe that always warns is a probe nobody reads. The standing false positive disables precisely the signal it was built to raise.

Before / after — actual output on this host

Live state, unchanged by this PR:

$ cat workspace/hosts/Chis-Mac-mini/schedule-crons-stamp.json
{"ts": 1785607670, "registered": 8, "config_total": 9}

CronList shows 8 registered; the single absentee is the 0 0 31 2 * entry.

At origin/main (badd2a6) — the new tests run against unmodified code:

FAIL: test_parked_entry_is_not_expected
AssertionError: 'warn' != 'ok' : only 1/2 session cron(s) registered at last /schedule-crons

Ran 17 tests
FAILED (failures=2, errors=2)

The failure text reproduces the production symptom verbatim.

At this head:

Ran 17 tests
OK

Full health-check suite, judged by exit code (43 files): 0 failures.

The fix

session_owned() drops an entry only when its day-of-month/month pair is genuinely impossible. Conservative in both directions:

• Day-of-week must be unrestricted. cron ORs day-of-month with day-of-week, so 0 0 31 2 MON still fires on Mondays and is correctly kept.
• Anything unparseable is schedulable. Names (FEB), L/W, wrong field counts, bad steps all return "can fire", so a parser gap can never invent a never-fires verdict.
• A real shortfall still warns — covered by its own test, where the parked entry is excluded from both sides of the ratio and a genuine 1/2 still trips.

Feb 29 is treated as reachable (leap years), so only truly impossible pairs are excluded.

Scope

One function plus a module-level helper in src/health-check.py, and tests. Touches no other probe.

Four of my open PRs also edit src/health-check.py (#2494, #2471, #2316, #2160). I checked their hunk ranges before writing: they land at lines ~43, ~1371–1511, ~1579–1655, and ~2487–2840. This change sits at ~720–818 and overlaps none of them.

Note on the test file

The new cases were initially appended after the file's if __name__ == "__main__": unittest.main() block, where they were defined but never collected — 13 ran while 17 were defined, and the suite looked green against both fixed and unfixed code. Caught by the two-identical-outputs check and relocated above the guard. The failing-at-main output above is from the corrected file.

@sonichi flagging you per usual.

[github-actions]

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 71%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

• src/health-check.py (100%)

Summary

• Total: 48 lines
• Missing: 0 lines
• Coverage: 100%

[qingyun-wu]

Reviewed exact head cc6ffa55cb7b760d5733e927bc06138a466b7948.

[P1 blocker] src/health-check.py:829-832 classifies every impossible calendar expression as parked, but the code never checks that the entry is actually disabled. That makes the registration guard hide an active typo—the exact silent-miss class this check exists to surface.

Exact reproduction with one registered main loop plus an active entry named monthly-report at 0 0 31 2 *:

• parent badd2a64: warn — only 1/2 session cron(s) registered
• this head cc6ffa55: ok — 1 session cron(s) stamped registered this boot

The supplied test only uses a DISABLED-named entry, so it does not cover this distinction. Please scope the exclusion to an explicit disabled signal (for example a documented schema field, or at minimum the established disabled-name convention) and add a negative control proving that an active impossible schedule still warns.

Worst-case disruption: an owner mistypes an active monthly schedule, CronCreate omits it, and health-check reports green forever. Changes requested; not merge-ready.

Reviewer-run checks: focused suite 17/17, Python 3.9 compilation, Ruff, diff hygiene, and the REVIEW.md hardcoded-path gate pass. Clean-install and diff-coverage checks were still running at review time.

[sonichi]

You're right, and the blocker is real. Fixed at 8f717369.

Excluding on the impossible date alone was the error. 0 0 31 2 * is equally the signature of a parked job and of someone who meant "the 31st, monthly" and wrote February. CronCreate omits it either way, so dropping it from expected too made health-check read green forever — the exact silent-miss class this check exists to surface.

The discriminator should have been "deliberately parked", not "impossible date".

Fix

Exclusion now requires both signals: an explicit parked marker — disabled: true, or the established DISABLED-in-name convention this host already uses — and an unregistrable expression. An impossible date with no marker still warns.

Your reproduction, at three heads

One live main-loop plus an active monthly-report at 0 0 31 2 *:

parent badd2a64 : warn — only 1/2 session cron(s) registered
cc6ffa55 (bad)  : ok   — 1 session cron(s) stamped registered this boot
this head       : warn — only 1/2 session cron(s) registered   <- matches parent

Fixture axis, not the single case

You noted my fixture only used a DISABLED-named entry, so it couldn't express the distinction. Rather than add your one case, I widened the axis — a 6-case matrix over {DISABLED-name, disabled: true field, no marker} × {impossible, registrable}:

entry	cron	excluded?
wire-DISABLED-2026	0 0 31 2 *	yes
disabled: true	0 0 31 2 *	yes
monthly-report (no marker)	0 0 31 2 *	no — typo
wire-DISABLED-2026	0 4 * * *	no — registrable
disabled: true	0 4 * * *	no — registrable
ordinary	0 4 * * *	no

Only marked-and-unregistrable is excluded.

Diff coverage

Also closed the 83.7% gap (missing 736, 742, 746-749, 755) with cases driving each branch of the field expander: ranges, a valid step, empty list segments, inverted and out-of-range bounds.

One disclosure on how I verified that. coverage collects nothing in this environment — its own positive control on a trivial script reports no-data-collected — so any local percentage I quoted would have been a false reading. Instead I called _cron_field_values directly and confirmed each previously-missing line is reached:

line 736  empty list segment      _cron_field_values('31,')    -> None
line 742  valid step assignment   _cron_field_values('1-31/5') -> set(7)
lines 746-749  range branch       _cron_field_values('30-31')  -> {30, 31}
line 755  above field max         _cron_field_values('40')     -> None
line 755  inverted range          _cron_field_values('20-10')  -> None
line 755  below field min         _cron_field_values('0')      -> None

CI's measurement is the authority here; I'm claiming the branches are driven, not a percentage.

Focused suite 20/20; full health-check suite 43 files, 0 failures by exit code.

[qingyun-wu]

Reviewed exact head 8f71736945f6fc49a0a01d5a5ef8897a3b9342b0.

The prior false-green blocker is resolved. A cron is excluded only when it has an explicit disabled marker and its expression cannot fire; an active monthly-report at February 31 remains expected and still warns. Reviewer-run focused suite passed 20/20, including the six-case marker/schedulability matrix and parser fail-open controls; Python compilation, diff hygiene, REVIEW.md path checks, CI, and CLA are green.

Worst case was an active schedule typo being silently omitted while health-check reported OK forever. The current discriminator and negative control prevent that. Ready to merge from this review lane.

[john-the-dev]

Approved on exact head 8f717369 after independent failure-mode review.

Current main e4276b20 still reproduces the real false warning for one live cron plus a marked parked 0 0 31 2 * entry: only 1/2 session cron(s) registered. On this head, the focused suite passes 20/20, including the six-case {marker/no-marker} × {possible/impossible} matrix, active-typo negative control, genuine-shortfall control, day-of-week OR semantics, leap-day handling, and parser fail-open cases. Python compilation, diff hygiene, and the REVIEW.md hardcoded-path scan pass; every exact-head CI/coverage/CLA gate is green.

The activated health path now excludes only explicitly parked AND provably unregistrable entries. Worst-case disruption was hiding an active cron typo forever; the no-marker negative control keeps that warning intact. LGTM, non-blocking.

Not merge-ready yet: the branch is behind current main and maintainerCanModify is false, so the author must refresh it; after refresh, revalidate full CI/CLA and two qualifying current-head approvals before merge.

[github-actions]

@cla-assistant check