fix(gateway): retry failed kanban notifications

[coderlaoma]

Summary

• Treat SendResult(success=False) from platform adapters as a Kanban notifier delivery failure.
• Rewind claimed notification cursors on failed sends so terminal events remain retryable.
• Keep subscriptions after repeated send failures instead of silently dropping them.

Why

A platform adapter can report API-level send failures by returning SendResult(success=False) without raising. The notifier previously treated that as delivered, allowing blocked/completed Kanban notifications to be silently consumed. Repeated raised send failures could also drop the subscription, leaving no retry path.

Test Plan

• uv run --with pytest --with pytest-asyncio --with pyyaml python -m pytest tests/gateway/test_kanban_notifier.py -q -o 'addopts='

Platforms tested

• Linux — Ubuntu 24.04.4 LTS, Python 3.11 (CPython, uv venv). 10/10 tests in tests/gateway/test_kanban_notifier.py pass.

Manual verification

Beyond the unit tests, this change has been running on a live multi-profile hermes gateway run deployment since 2026-06-12. gateway.log shows the new path exercised in production against the WeChat/iLink adapter under real rate-limiting: subscriptions for tasks t_9a53bfd4 / t_49129a54 hit 11–14 consecutive SendResult-level failures, backed off (2560s → capped 3600s) instead of being dropped, and successfully delivered the completed-event notification once the rate limit cleared — exactly the "retryable instead of silently consumed" behavior this PR targets.

Cross-platform / Security impact

• No cross-platform impact: pure-Python timer logic (time.monotonic), no file I/O / process / terminal changes; check-windows-footguns.py N/A.
• No security impact: no shell / path / privilege surface touched.

[liuhao1024]

Review: Unbounded retry loop for permanently dead chats

The PR removes the subscription-dropping mechanism (_kanban_unsub + sub_fail_counts.pop) when fails >= MAX_SEND_FAILURES, replacing it with a warning log. However, MAX_SEND_FAILURES = 3 is still defined and the counter still increments — it just no longer triggers any action.

Impact: For permanently unreachable chats (bot kicked, channel deleted, etc.), the notifier will retry adapter.send() every 5 seconds indefinitely. The warning log fires every tick once fails >= 3, generating unbounded log spam.

Suggestion: Keep the cursor-rewind behavior for the first N retries (transient failures), but add a backoff or dead-letter mechanism after repeated failures:

if fails >= MAX_SEND_FAILURES:
    # Exponential backoff: skip ticks proportional to failure count
    if fails % (2 ** min(fails - MAX_SEND_FAILURES, 6)) != 0:
        break
    logger.warning(
        "kanban notifier: subscription %s on %s has %d "
        "consecutive failures; retrying with backoff",
        sub["task_id"], platform_str, fails,
    )

This preserves the "never silently drop" intent while bounding retry frequency for dead chats.

[coderlaoma]

Thanks for catching this — you're right that dropping the unsubscribe path left MAX_SEND_FAILURES as a dead counter, and a permanently dead chat would hit adapter.send() plus a warning line every 5s tick forever. Fixed in 45459de.

What changed:

• Replaced the bare failure counter with per-subscription state: {fails, next_retry_at}.
• First MAX_SEND_FAILURES (3) failures retry on the normal tick cadence, preserving fast recovery from transient errors.
• Beyond that, each failure doubles the retry delay (interval * 2^(fails - MAX + 1), capped at 1h). For a dead chat this means the send attempts and log lines decay to ~1/hour steady state instead of every 5s.
• The backoff gate sits before the cursor claim in _collect, so suppressed ticks do zero work: no claim, no rewind churn on the DB, no log output.
• Any successful send pops the state, restoring normal cadence — so the no-silent-drop semantics of the PR are intact: the subscription and the unseen event survive indefinitely, and delivery recovers as soon as the chat does.

Why not the tick-modulo gate from the suggestion: fails only increments on actual send attempts, so fails % (2 ** min(fails - MAX_SEND_FAILURES, 6)) != 0 → break deadlocks once fails lands on a non-zero residue — e.g. at fails=5, 5 % 4 != 0 skips the tick, but skipping never changes fails, so every subsequent tick re-evaluates the same expression and breaks forever. That would silently stop retrying, which is exactly what this PR set out to avoid. Backing off on a time.monotonic() deadline avoids that class of bug entirely.

Added two tests: test_kanban_notifier_backs_off_after_repeated_send_failures (window suppresses sends entirely, retry resumes after expiry, backoff grows) and test_kanban_notifier_recovers_after_backoff_when_chat_comes_back (success during a backoff retry delivers and clears state). Full tests/gateway/ suite run locally — the 10 failures are pre-existing on the base branch (telegram markdown-escaping / wecom callback / agent-cache mtime), unrelated to this change.

A proper dead-letter path (adapters classifying permanent errors like 403 bot-kicked in SendResult.error, then unsubscribing immediately) would be the real fix for dead chats, but it needs per-platform adapter changes — happy to follow up in a separate PR if you think it's worth it.

[liuhao1024]

Verification: Code Review ✅

Reviewed the diff — the implementation is solid:

• Exponential backoff is correctly bounded: delay = min(interval * (2 ** (fails - MAX_SEND_FAILURES + 1)), MAX_RETRY_DELAY) with MAX_RETRY_DELAY = 3600.0 (1 hour cap). This prevents both infinite retry spam and unbounded backoff growth.

• Backoff skips before claiming: The if time.monotonic() < fail_state["next_retry_at"] check happens before claim_unseen_events_for_sub, so the cursor is untouched during backoff windows. This avoids claim/rewind churn on dead chats — correct design.

• SendResult(success=False) handled: The new if getattr(send_result, "success", True) is False check converts API-level delivery failures (which some adapters return instead of raising) into RuntimeError so they enter the same failure-counting path. Good catch.

• Subscription never silently dropped: The old _kanban_unsub call on repeated failures is removed. The subscription stays alive and the event stays unseen — the user eventually gets their blocked/completed notification when the chat recovers. This is the correct "never silently consume a terminal notification" policy.

• Test coverage: 4 new tests cover SendResult(success=False), repeated-failure subscription retention, backoff window suppression, and post-backoff recovery. The state["next_retry_at"] = time.monotonic() - 1 trick to expire the window in tests is clean.

• No issues found. The design correctly balances "never drop notifications" with "don't hammer dead chats."

[coderlaoma]

8 workflows awaiting approval

Hi @liuhao1024 , thanks again for reviewing the PR.

I noticed that the Checks tab shows 8 workflows awaiting approval for this PR, so CI has not started yet. Could you please help approve the workflow runs when you have a chance, or let me know if there is anything I need to do on my side?

Thanks!

[coderlaoma]

@alt-glitch When you have a moment, could you approve the pending workflow runs for this PR? CI hasn't been triggered yet since it's a first-time contribution from a fork. The change is review-approved and ready — thanks!