From 9c68d12079539cffa475ed4171e45da6866243d0 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sun, 10 May 2026 13:18:44 -0700 Subject: [PATCH] test(kanban): cover send-exception rewind + drop noisy success log to debug Two follow-up improvements to the previous commit's notifier dedup work. 1. Add a regression test for the send-exception rewind path. The contributor's PR included a test for the adapter-disconnect path (test_kanban_notifier_rewinds_claim_if_adapter_disconnects, where adapter is None at delivery time), but not for the "adapter is connected, send() raises" path that fires inside the inner try/except at gateway/run.py:4314. The new test (test_kanban_notifier_rewinds_claim_on_send_exception) uses a FailingAdapter that always raises and confirms (a) send was actually attempted, (b) the claim was rewound, (c) the next call to unseen_events_for_sub still returns the event for retry. 2. Drop the per-delivery success log from INFO to DEBUG. A busy board on a multi-platform gateway can produce hundreds of these per day; that's gateway.log noise that obscures real warnings. Failure paths stay at WARNING (where you'd want to look when something's wrong) so we don't lose visibility into transient send issues. --- gateway/run.py | 2 +- tests/gateway/test_kanban_notifier.py | 36 +++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/gateway/run.py b/gateway/run.py index c9400fdfdd9..cbb167dcac0 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -4303,7 +4303,7 @@ class GatewayRunner: await adapter.send( sub["chat_id"], msg, metadata=metadata, ) - logger.info( + logger.debug( "kanban notifier: delivered %s event for %s to %s/%s on board %s", kind, sub["task_id"], platform_str, sub["chat_id"], board_slug, ) diff --git a/tests/gateway/test_kanban_notifier.py b/tests/gateway/test_kanban_notifier.py index f09c6acb967..a7a5fcbb1ee 100644 --- a/tests/gateway/test_kanban_notifier.py +++ b/tests/gateway/test_kanban_notifier.py @@ -136,3 +136,39 @@ def test_kanban_db_path_is_test_isolated_from_real_home(): assert kb.kanban_db_path().resolve().is_relative_to(hermes_home.resolve()) assert kb.kanban_db_path().resolve() != production_db.resolve() + + +class FailingAdapter: + """Adapter whose send() always raises, simulating a transient send error.""" + + def __init__(self): + self.attempts = 0 + + async def send(self, chat_id, text, metadata=None): + self.attempts += 1 + raise RuntimeError("simulated send failure") + + +def test_kanban_notifier_rewinds_claim_on_send_exception(tmp_path, monkeypatch): + """A raising adapter rewinds the claim so the next tick can retry. + + This is the second rewind path (distinct from the adapter-disconnect path + in test_kanban_notifier_rewinds_claim_if_adapter_disconnects). Here the + adapter is connected and the send call actually fires; the claim must + still rewind so the event isn't lost when send() raises mid-tick. + """ + db_path = tmp_path / "send-failure.db" + monkeypatch.setenv("HERMES_KANBAN_DB", str(db_path)) + kb.init_db() + tid = _create_completed_subscription() + + adapter = FailingAdapter() + runner = _make_runner(adapter) + + asyncio.run(_run_one_notifier_tick(monkeypatch, runner)) + + # Send was attempted (so we exercised the failure path, not just the + # disconnect path) and the claim was rewound — the unseen-events query + # still returns the event for retry on the next tick. + assert adapter.attempts >= 1, "send should have been attempted at least once" + assert [ev.kind for ev in _unseen_terminal_events(tid)] == ["completed"]