mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-27 17:58:07 +00:00
fix(kanban): widen notifier pre-filter to secondary-profile platforms
_collect()'s active_platforms pre-filter was derived solely from self.adapters (the default profile), so a subscription owned by a secondary profile on a platform the default profile never connected (e.g. beta owns discord, default has no discord adapter at all) was skipped before claim_unseen_events_for_sub ever ran. Unlike the disconnected-adapter path, an unclaimed event is never rewound, so this was a permanent, silent notification/wake loss — directly contradicting the point of routing notifications via the owning profile (c69643026/b225b30d0). Same cross-profile-adapter-lookup bug class the delivery-side _authorization_adapter chokepoint already guards against, one gate earlier. The precise per-profile check still runs unchanged at delivery time, with its existing rewind-on-None safety net.
This commit is contained in:
parent
67826e3068
commit
dda6f0f63e
2 changed files with 67 additions and 0 deletions
|
|
@ -208,6 +208,24 @@ class GatewayKanbanWatchersMixin:
|
|||
getattr(platform, "value", str(platform)).lower()
|
||||
for platform in self.adapters.keys()
|
||||
}
|
||||
# Widen to every platform any secondary profile has live,
|
||||
# not just the default profile's. This is only a coarse
|
||||
# pre-filter to skip claiming events for subs nobody can
|
||||
# possibly deliver — the precise per-profile check (via
|
||||
# gateway/authz_mixin.py::_authorization_adapter, which
|
||||
# forbids default-profile fallback) still runs at delivery
|
||||
# time below, rewinding the claim if it resolves to None.
|
||||
# Without this, a subscription owned by a secondary
|
||||
# profile on a platform the DEFAULT profile never
|
||||
# connected (e.g. beta owns discord, default doesn't) was
|
||||
# dropped here before ever being claimed — no rewind
|
||||
# applies to an unclaimed event, so it silently never
|
||||
# retries.
|
||||
for _profile_adapter_map in getattr(self, "_profile_adapters", {}).values():
|
||||
active_platforms.update(
|
||||
getattr(platform, "value", str(platform)).lower()
|
||||
for platform in _profile_adapter_map.keys()
|
||||
)
|
||||
if not active_platforms:
|
||||
logger.debug("kanban notifier: no connected adapters; skipping tick")
|
||||
return deliveries
|
||||
|
|
|
|||
|
|
@ -416,6 +416,55 @@ def test_notifier_owning_profile_adapter_no_default_fallback(tmp_path, monkeypat
|
|||
assert [ev.kind for ev in _unseen_terminal_events_for(tid, "chat-beta")] == ["completed"]
|
||||
|
||||
|
||||
def test_notifier_claims_platform_only_a_secondary_profile_owns(tmp_path, monkeypatch):
|
||||
"""A subscription owned by a secondary profile on a platform the DEFAULT
|
||||
profile never connected must still be claimed and delivered.
|
||||
|
||||
Regression: the ``_collect()`` pre-filter built ``active_platforms``
|
||||
solely from ``self.adapters`` (the default profile). A sub owned by
|
||||
profile "beta" on "discord", where beta genuinely has a live discord
|
||||
adapter but the default profile has no discord adapter at all, was
|
||||
dropped by that pre-filter (``platform not in active_platforms``)
|
||||
before ``claim_unseen_events_for_sub`` ever ran — unlike the
|
||||
disconnected-adapter path, an unclaimed event is never rewound, so this
|
||||
was a permanent, silent notification loss, not a retryable one. This
|
||||
directly contradicts the feature's own purpose (routing notifications
|
||||
via the owning profile), and is the same cross-profile-adapter-lookup
|
||||
class the delivery-side chokepoint in
|
||||
``test_notifier_owning_profile_adapter_no_default_fallback`` already
|
||||
guards — just one gate earlier.
|
||||
"""
|
||||
db_path = tmp_path / "secondary-only-platform.db"
|
||||
monkeypatch.setenv("HERMES_KANBAN_DB", str(db_path))
|
||||
kb.init_db()
|
||||
|
||||
conn = kb.connect()
|
||||
try:
|
||||
tid = kb.create_task(conn, title="owned by beta on discord", assignee="worker")
|
||||
kb.add_notify_sub(
|
||||
conn, task_id=tid, platform="discord", chat_id="chat-beta",
|
||||
notifier_profile="beta",
|
||||
)
|
||||
kb.complete_task(conn, tid, summary="done")
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
beta_adapter = RecordingAdapter()
|
||||
runner = GatewayRunner.__new__(GatewayRunner)
|
||||
runner._running = True
|
||||
# Default profile has NO discord adapter at all.
|
||||
runner.adapters = {Platform.TELEGRAM: RecordingAdapter()}
|
||||
# Secondary profile "beta" has a live discord adapter.
|
||||
runner._profile_adapters = {"beta": {Platform.DISCORD: beta_adapter}}
|
||||
runner._kanban_sub_fail_counts = {}
|
||||
|
||||
asyncio.run(_run_one_notifier_tick(monkeypatch, runner))
|
||||
|
||||
assert len(beta_adapter.sent) == 1, (
|
||||
f"beta's discord adapter should have received the notification; got {beta_adapter.sent!r}"
|
||||
)
|
||||
|
||||
|
||||
def test_notifier_wakeup_uses_subscription_chat_type(tmp_path, monkeypatch):
|
||||
db_path = tmp_path / "chat-type-wakeup.db"
|
||||
monkeypatch.setenv("HERMES_KANBAN_DB", str(db_path))
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue