Skip to content

fix(kanban): widen notifier pre-filter to secondary-profile platforms - #486

Open
hashbender wants to merge 1 commit into
mainfrom
mirror/pr-56589
Open

fix(kanban): widen notifier pre-filter to secondary-profile platforms#486
hashbender wants to merge 1 commit into
mainfrom
mirror/pr-56589

Conversation

@hashbender

Copy link
Copy Markdown
Owner

Summary

  • Follow-up to the just-merged kanban profile-routing salvage (feat(kanban): route notifications via owning profile + wake creator agent NousResearch/hermes-agent#54872 / fix(kanban): route notifications via owning profile + wake creator agent (salvage #54872) NousResearch/hermes-agent#56531 / security(browser): re-check private-network guard after browser_back navigation NousResearch/hermes-agent#56526's sibling review pass), which fixed the delivery-side adapter lookup to route through the shared _authorization_adapter chokepoint (gateway/authz_mixin.py) so a secondary profile's subscription is never silently delivered by the default profile's bot.
  • That fix left an earlier gate untouched: _kanban_notifier_watcher's _collect() pre-filters subscriptions by active_platforms, a set built only from self.adapters (the default/active profile). A subscription owned by a secondary profile on a platform the default profile never connected at all (e.g. profile beta has a live Discord adapter, but the default profile has no Discord adapter) is dropped by this pre-filter before claim_unseen_events_for_sub is ever called.
  • Unlike the already-guarded disconnected-adapter path (which rewinds an already-claimed event so it retries next tick), an unclaimed event here has nothing to rewind — the subscription is skipped every tick, forever, as long as the default profile's adapter set doesn't change. This is a silent, permanent loss of the task's terminal notification and creator-wake, directly contradicting the stated purpose of routing notifications via the owning profile.
  • Fix: widen active_platforms to the union of self.adapters and every platform present across self._profile_adapters.values(). This is intentionally a coarse pre-filter — the precise per-profile chokepoint (_authorization_adapter, which still forbids default-profile fallback) runs unchanged at delivery time a few lines below, with its existing rewind-on-None safety net for any sub that over-admits here but doesn't actually have a live adapter.

Test plan

  • Added test_notifier_claims_platform_only_a_secondary_profile_owns to tests/gateway/test_kanban_notifier.py, reproducing the exact scenario (default profile has telegram only, secondary profile beta has a live discord adapter, sub is owned by beta on discord).
  • Mutation-verified: reverting the kanban_watchers.py change makes the new test fail (assert 0 == 1, adapter never receives the notification); passes with the fix.
  • Ran the full existing tests/gateway/test_kanban_notifier.py suite (8 tests, including test_notifier_owning_profile_adapter_no_default_fallback) — all pass, no regressions.
  • Ran the broader tests/gateway/ -k kanban suite (22 passed, 2 skipped) — no regressions.

Mirror-of: NousResearch#56589
NousResearch#56589

@tenki-reviewer

tenki-reviewer Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review Complete
No issues found!

Risk: 🟢 Low (18/100) — no findings · 192 LOC across 2 files


Fixes a cross-profile kanban notification mis-delivery bug by routing adapter resolution through the shared authorization chokepoint and widening the active-platforms pre-filter to include secondary profiles. Adds two targeted integration tests covering the no-fallback and secondary-profile-only-platform scenarios.

Files Reviewed (2 files)
gateway/kanban_watchers.py
tests/gateway/test_kanban_notifier.py

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant