fix: deduplicate kanban notifications for blocked/gave_up states by jelrod27 · Pull Request #21398 · NousResearch/hermes-agent

jelrod27 · 2026-05-07T16:03:50Z

Problem

The kanban notifier re-fires the same blocked/gave_up/crashed/timed_out notifications on every 5-second tick, creating Telegram spam that looks like a runaway loop.

Root Cause

After delivering a terminal event, the notifier unsubscribed the subscription, deleting its row (including the cursor). If the unsub succeeded, the subscription was gone — any future state transitions on that task would have no subscription to deliver through. If the unsub failed (SQLite WAL contention, transient I/O error), the subscription survived with a stale cursor (last_event_id=0), causing the same event to be re-fetched and re-delivered on every tick indefinitely.

Fix

Stop unsubscribing on terminal event kinds (blocked, gave_up, crashed, timed_out). Only remove the subscription when the task reaches a truly final status (done / archived).

For the intermediate terminal states, the subscription stays alive and the cursor mechanism deduplicates naturally — events with id <= last_event_id are never re-fetched by unseen_events_for_sub. This makes the dedup idempotent:

Before: unsub on terminal event → cursor deleted → if unsub fails, re-fires forever
After: advance cursor on terminal event → cursor persists → next tick sees no new events → silence

This also fixes a secondary bug: if a task was blocked, unblocked, then blocked again, the first blocked notification would unsub the subscription, so the second blocked event would never be delivered. Now the subscription survives and the user gets both notifications.

Changes

gateway/run.py: Remove TERMINAL_EVENT_KINDS alias and the event_terminal check from the unsubscription condition. Only unsub when task.status in ("done", "archived").
Updated comments to document the fix rationale.

Verification

All 152 kanban tests pass (1 pre-existing failure unrelated to this change)
Manual: set 5 kanban tasks to blocked, watched Telegram for 5 minutes — zero re-fires after the initial notification
Manual: transitioned a blocked task to ready then back to blocked — single notification for each transition, no spam

… to 2 Schema: tasks.max_retries INTEGER nullable, no column default. Migration: PRAGMA table_info guard, appended after skills column. Constant: DEFAULT_FAILURE_LIMIT 5 -> 2. Breaker: effective_limit resolution in _record_task_failure(). Task dataclass: max_retries field + _row_to_task handling. create_task: max_retries kwarg + INSERT column. gave_up event: effective_limit + limit_source in payload.

- hermes kanban create --max-retries N overrides per-task retry cap - hermes kanban show displays effective limit and source (task/default) - _task_to_dict includes max_retries in JSON output

1. test_max_retries_per_task_overrides_default: task with max_retries=1 2. test_config_default_used_when_no_per_task: default limit=2 behavior 3. test_per_task_overrides_config_and_default: max_retries=10 blocks at 10, not 2 4. test_gave_up_event_fires_at_cap: event payload with effective_limit/limit_source 5. test_existing_consecutive_failures_tests_still_pass: regression

…es-override

The kanban notifier was re-firing the same blocked/gave_up/crashed/timed_out notifications on every 5-second tick. Root cause: after delivering a terminal event, the notifier unsubscribed the subscription, deleting its cursor. If the unsub failed (WAL contention, transient error), the subscription survived with a stale cursor, and the next tick would re-deliver the same event. Even when the unsub succeeded, the subscription was gone. If the task later transitioned to a different state (e.g., blocked -> unblocked -> blocked again), a new subscription would start at cursor=0, re-delivering all past events. Fix: stop unsubscribing on terminal event kinds. Only remove the subscription when the task reaches a truly final status (done/archived). For blocked, gave_up, crashed, and timed_out, the subscription stays alive and the cursor mechanism deduplicates naturally -- events with id <= last_event_id are never re-fetched. This makes the dedup idempotent and eliminates the re-fire bug. The old concern about subscriptions leaking forever on blocked tasks is moot: blocked tasks will eventually be unblocked (transitioning to ready/running) or archived, at which point the subscription is cleaned up.

…-event tests Follow-up to the previous commit's notifier behavior change. Two test fixes: 1. `tests/gateway/test_kanban_notifier.py` gains `test_notifier_redelivers_same_kind_on_dispatch_cycle` — pins the new contract directly: a task that crashes, gets reclaimed, and crashes again notifies the user BOTH times. Before #21398 the second crash silently dropped because the subscription was already deleted. 2. `tests/hermes_cli/test_kanban_notify.py:: test_notifier_unsubs_after_abnormal_events[gave_up|crashed|timed_out]` is flipped. Those tests were added in the salvage of #22941 and asserted the OLD behavior (subscription deleted after gave_up / crashed / timed_out). They're now obsolete — the new contract is "subscription survives a non-final terminal event so retries reach the user." Updated docstring + asserts; the cursor-advance check is added to confirm the dedup mechanism still works. The `test_notifier_unsubs_after_completed_event` test stays untouched because `completed` IS still a terminal event that triggers unsub (the task hits `done` status, which is handled by the `task_terminal` branch in the notifier loop).

teknium1 · 2026-05-10T21:28:13Z

Salvage merged via PR #23423 (rebase) — your gateway/run.py change shipped on main with your authorship preserved (re-attributed during salvage from justinelrod@Justin-Agent.local to the GitHub-noreply form so release notes credit your account). AUTHOR_MAP entry added.

Carve-out: the salvage took just the substantive notifier fix (commit 814aa6f). The earlier 4 commits in your branch (feat(kanban): max_retries per-task override and the related CLI flag + 5 tests) were NOT carried — those landed independently via PR #21330 (commit ac51c4c1a) on May 7. Your branch had merged main and was carrying duplicates as a result, which is why gh pr view showed +140 LOC of unrelated test additions.

On the bug framing: your PR described the symptom as "every-tick spam from a failed unsub leaving a stale cursor=0." That specific mechanism doesn't actually fire on current main because the cursor advance precedes the unsub call (and post-#23401 they're inside the same BEGIN IMMEDIATE write txn). The REAL bug is the cycle bug: a worker that crashes, gets reclaimed, runs again, and crashes a second time would only notify on the first crash because the subscription was deleted after the first event. Same shape as the reblock-after-unblock cycle PR #22941 fixed for blocked. The fix you wrote is correct for that bug; I just rewrote the surrounding comment to match.

Improvements during salvage:

Added a focused regression test (test_notifier_redelivers_same_kind_on_dispatch_cycle) that pins the new contract directly: two crashes, two deliveries, subscription survives between them.
Flipped the 3 stale tests in tests/hermes_cli/test_kanban_notify.py::test_notifier_unsubs_after_abnormal_events that were added during PR fix(kanban): reblock task notification (salvage #22193) #22941's salvage but expected the OLD behavior — now they correctly expect the subscription to survive a non-final terminal event with the cursor advanced.

Thanks @jelrod27 — clean fix shape, the bug was real, just minor cleanup on the framing.
#23423

…-event tests Follow-up to the previous commit's notifier behavior change. Two test fixes: 1. `tests/gateway/test_kanban_notifier.py` gains `test_notifier_redelivers_same_kind_on_dispatch_cycle` — pins the new contract directly: a task that crashes, gets reclaimed, and crashes again notifies the user BOTH times. Before NousResearch#21398 the second crash silently dropped because the subscription was already deleted. 2. `tests/hermes_cli/test_kanban_notify.py:: test_notifier_unsubs_after_abnormal_events[gave_up|crashed|timed_out]` is flipped. Those tests were added in the salvage of NousResearch#22941 and asserted the OLD behavior (subscription deleted after gave_up / crashed / timed_out). They're now obsolete — the new contract is "subscription survives a non-final terminal event so retries reach the user." Updated docstring + asserts; the cursor-advance check is added to confirm the dedup mechanism still works. The `test_notifier_unsubs_after_completed_event` test stays untouched because `completed` IS still a terminal event that triggers unsub (the task hits `done` status, which is handled by the `task_terminal` branch in the notifier loop).

Justin Elrod and others added 7 commits May 6, 2026 17:27

feat(kanban): CLI --max-retries flag and show display

05a75be

- hermes kanban create --max-retries N overrides per-task retry cap - hermes kanban show displays effective limit and source (task/default) - _task_to_dict includes max_retries in JSON output

Merge pull request NousResearch#1 from jelrod27/feat/kanban-max-retri…

d1b472a

…es-override

Merge remote-tracking branch 'origin/main'

7cab312

merge: resolve conflicts by taking upstream changes

43e077f

alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists labels May 7, 2026

teknium1 mentioned this pull request May 10, 2026

fix(kanban): keep notifier subscriptions alive across retry cycles (salvage #21398) #23423

Merged

teknium1 added a commit that referenced this pull request May 10, 2026

chore: AUTHOR_MAP entry for jelrod27 (#21398)

b38b100

teknium1 closed this May 10, 2026

JinyuID pushed a commit to JinyuID/hermes-agent that referenced this pull request May 11, 2026

chore: AUTHOR_MAP entry for jelrod27 (NousResearch#21398)

fa03714

02356abc pushed a commit to 02356abc/hermes-agent that referenced this pull request May 14, 2026

chore: AUTHOR_MAP entry for jelrod27 (NousResearch#21398)

f63e318

jsboige pushed a commit to jsboige/hermes-agent that referenced this pull request May 14, 2026

chore: AUTHOR_MAP entry for jelrod27 (NousResearch#21398)

8b4242e

AlexFoxD pushed a commit to AlexFoxD/hermes-agent that referenced this pull request May 21, 2026

chore: AUTHOR_MAP entry for jelrod27 (NousResearch#21398)

f1df26f

gweeteve pushed a commit to gweeteve/hermes-agent that referenced this pull request Jun 2, 2026

chore: AUTHOR_MAP entry for jelrod27 (NousResearch#21398)

d6a7ff0

Seven74AI pushed a commit to Seven74AI/hermes-agent that referenced this pull request Jun 13, 2026

chore: AUTHOR_MAP entry for jelrod27 (NousResearch#21398)

c46aecb

T02200059 pushed a commit to T02200059/hermes-agent that referenced this pull request Jun 18, 2026

chore: AUTHOR_MAP entry for jelrod27 (NousResearch#21398)

78207c3

Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026

chore: AUTHOR_MAP entry for jelrod27 (NousResearch#21398)

3cfdac7

Navigation Menu

Uh oh!

fix: deduplicate kanban notifications for blocked/gave_up states - #21398

fix: deduplicate kanban notifications for blocked/gave_up states#21398
jelrod27 wants to merge 7 commits into
NousResearch:mainfrom
jelrod27:fix/notification-dedupe-on-blocked-state

jelrod27 commented May 7, 2026

Uh oh!

teknium1 commented May 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

jelrod27 commented May 7, 2026

Problem

Root Cause

Fix

Changes

Verification

Uh oh!

teknium1 commented May 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants