Compare commits

...

1 commit

Author SHA1 Message Date
Matt Johnson
18208d2e8f fix(dispatcher): qualify region-cooldown key by channel type
Section 1.5 (region_routes matrix branch) armed and checked the
per-region cooldown key as (toggle, category, region) with no
channel-type component. _chans always inserts mesh_broadcast before
meshcore_broadcast (insertion order in the per-cell append loop), so
for any matched cell with BOTH mt and mc populated, the mesh_broadcast
send armed the cooldown key first; the very next iteration checked
that SAME key for meshcore_broadcast and saw it as freshly cooled
down, dropping it every time. Net effect: meshcore_broadcast never
succeeded via the matrix branch whenever cooldown_seconds > 0 (true
for weather/roads/fire, all 300s), so it never armed its own
dedup/cooldown state either -- a silent, permanent MC blackout for
every region-routed family. Confirmed live: dispatcher_dedup had 555
rows, zero meshcore_broadcast; mesh_broadcasts_out was 120:4 MT:MC for
nws_alerts and 98:8 for traffic_events over 10 days (the few MC rows
that got through came from a different, non-matrix code path). fires'
98:77 near-1:1 ratio is not evidence the matrix branch worked for
fire -- those MC sends are dominated by the cooldown-exempt scheduled
reminder path (dispatch_scheduled_fire_broadcast); fire's own live
event-driven path has the identical latent bug, just masked.

Fix folds ch_type into the region string (mirrors the existing
_cd_suffix convention) so mesh_broadcast and meshcore_broadcast get
independent cooldown windows. Kept the cooldown key a 3-tuple
(instead of widening to 4) to avoid a dispatcher_cooldowns schema
migration -- _persist_cooldown() and the boot-restore SELECT are both
hard-coded to (toggle, category, region).

Verified in isolation (no live/deployed behavior change, no
transmit): a fresh Dispatcher built from the live production config
now dispatches both mesh_broadcast and meshcore_broadcast for
weather/roads/fire matched cells under a 300s cooldown. Added two
regression tests covering the gap that let this ship untested: no
existing test combined cooldown_seconds > 0 with a cell that has BOTH
mt and mc populated (test_cell_match_routes_mt_and_mc uses the
cooldown_s=0 default; test_per_region_cooldown_independence uses
cooldown_s=300 but with mc=None on every cell).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-11 14:59:20 +00:00
2 changed files with 88 additions and 2 deletions

View file

@ -501,13 +501,28 @@ class Dispatcher:
# Per-region cooldown check — skip channel only when EVERY
# region feeding it is still within its cooldown window.
# BUGFIX: _rk MUST be qualified by _ch_type. _chans iterates
# mesh_broadcast before meshcore_broadcast (insertion order,
# see the append loop above), so without the channel-type
# suffix the MT send for this SAME event armed this SAME key
# a few lines up -- MC's check then always sees a fresh
# last_fired_at and is dropped as "cooled down" on literally
# every event. That's why dispatcher_dedup had zero
# meshcore_broadcast rows despite mc_enabled+mc being
# correctly configured: MC never got far enough to succeed
# and arm its own dedup entry. Keeping _rk a 3-tuple (folding
# ch_type into the region string, mirroring the existing
# _cd_suffix convention) avoids a dispatcher_cooldowns schema
# migration -- _persist_cooldown()/the restore SELECT are
# both hard-coded to (toggle, category, region).
if _cooldown_s > 0:
_all_cooled = True
for _rg in _regions:
_rk = (
getattr(tog, "name", "") or fam,
event.category,
_rg + ("|" + _cd_suffix if _cd_suffix else ""),
_rg + ("|" + _cd_suffix if _cd_suffix else "")
+ "|" + _ch_type,
)
_last = self._toggle_cooldown.get(_rk)
if _last is None or (_now - _last) >= _cooldown_s:
@ -577,7 +592,8 @@ class Dispatcher:
_rk = (
getattr(tog, "name", "") or fam,
event.category,
_rg + ("|" + _cd_suffix if _cd_suffix else ""),
_rg + ("|" + _cd_suffix if _cd_suffix else "")
+ "|" + _ch_type,
)
self._toggle_cooldown[_rk] = _commit_now
self._persist_cooldown(_rk, _commit_now, _cooldown_s)

View file

@ -900,3 +900,73 @@ def test_ptx_destination_mesh_broadcast_not_double_when_matrix_owns_mt():
assert all(r["broadcast_channel"] != 7 for r in rec), \
"the mesh_broadcast destination (ch7) must be filtered by _matrix_handled"
assert len(rec) == 1, "only the matrix MT send; no extra destination delivery"
# ==================================================================
# Regression: cooldown key must be transport-qualified (2026-07-11)
# ------------------------------------------------------------------
# The Section 1.5 per-region cooldown key was (toggle, category, region) with
# NO channel-type component. _chans always inserts mesh_broadcast before
# meshcore_broadcast (insertion order in the per-cell append loop), so for a
# cell with BOTH mt and mc populated, the loop over _chans.items() processed
# mesh_broadcast first, delivered it, and ARMED the shared cooldown key. The
# very next iteration (meshcore_broadcast) checked that SAME key -- now fresh
# -- and was dropped as "cooled down", every single time, for every event.
# Net effect: meshcore_broadcast never succeeded via the matrix branch when
# cooldown_seconds > 0, so it never armed its own dedup/cooldown state either
# -- a permanent, silent MC blackout for every region-routed family (weather,
# 511/roads; fire's live path too, masked because fire's MC delivery was
# dominated by the cooldown-exempt scheduled reminder path).
#
# No prior test caught this because test_cell_match_routes_mt_and_mc uses the
# _base_cfg default cooldown_s=0 (the `if _cooldown_s > 0:` gate is skipped
# entirely), and test_per_region_cooldown_independence uses cooldown_s=300
# but with mc=None on every cell (MT-only), so it never exercises a cell with
# BOTH transports populated under a live cooldown window.
def test_cooldown_does_not_cross_suppress_mt_and_mc():
"""A cell with BOTH mt and mc populated, under cooldown_seconds > 0, must
deliver BOTH transports on the same event -- MT arming its cooldown must
NOT cause MC's cooldown check (same toggle/category/region) to see itself
as already-cooled-down within the same dispatch call."""
cfg = _base_cfg(fam="fire", cooldown_s=300, min_severity="routine")
cfg.notifications.region_routes = _rr(cells={
"fire": {
"SCI": {"mt": 3, "mc": "sci-fires", "min_severity": "routine", "enabled": True},
}
})
ev = _ev(fam="fire", region="SCI")
_, rec = _dispatch(cfg, ev)
types = {r["delivery_type"] for r in rec}
assert "mesh_broadcast" in types, "must deliver via mesh_broadcast"
assert "meshcore_broadcast" in types, (
"must ALSO deliver via meshcore_broadcast -- MT's cooldown arm must "
"not cross-suppress MC on the same event/region"
)
assert len(rec) == 2, "exactly two deliveries (mt + mc), got %r" % (rec,)
def test_cooldown_independent_per_transport_across_events():
"""After a successful MT+MC dispatch, a SECOND event in the SAME region
within the cooldown window must be dropped for BOTH transports (cooldown
still functions), and each transport's cooldown key must be independently
keyed -- not just accidentally passing because both are simply always-on
or always-off together."""
cfg = _base_cfg(fam="fire", cooldown_s=300, min_severity="routine")
cfg.notifications.region_routes = _rr(cells={
"fire": {
"SCI": {"mt": 3, "mc": "sci-fires", "min_severity": "routine", "enabled": True},
}
})
d, rec = _make_dispatcher(cfg)
ev1 = _ev(fam="fire", region="SCI", eid="ev-1")
asyncio.run(d.dispatch(ev1))
assert len(rec) == 2, "first event: both transports deliver"
ev2 = _ev(fam="fire", region="SCI", eid="ev-2")
asyncio.run(d.dispatch(ev2))
# Still within cooldown window for BOTH transports -> no new deliveries.
assert len(rec) == 2, "second event within cooldown: no new deliveries, got %r" % (rec,)