Skip to content

fix(kanban): run notifier independently of gateway dispatch - #67673

Closed
DeliciousHouse wants to merge 2 commits into
NousResearch:mainfrom
DeliciousHouse:hermes-agent/t_857b6656-fix-native-kanban-notifier-only-mode-and
Closed

fix(kanban): run notifier independently of gateway dispatch#67673
DeliciousHouse wants to merge 2 commits into
NousResearch:mainfrom
DeliciousHouse:hermes-agent/t_857b6656-fix-native-kanban-notifier-only-mode-and

Conversation

@DeliciousHouse

@DeliciousHouse DeliciousHouse commented Jul 19, 2026

Copy link
Copy Markdown

Summary

  • keep native Kanban notification polling active when kanban.dispatch_in_gateway is false, without starting or invoking the gateway dispatcher
  • preserve multi-board/profile-routed delivery and add scheduled notifications
  • deliver complete blocked/scheduled decision briefs so ASK, WHY GATED, SCOPE, ROLLBACK, REPLY, and scheduled WINDOW fields survive beyond 160 characters

Root cause

The notifier watcher shared the dispatcher ownership gate and returned before polling subscriptions when in-gateway dispatch was disabled. Blocked reasons were also sliced to 160 characters, and scheduled was absent from the notifiable event set.

Verification

  • Windows: 31 passed across notifier, dispatcher-gate, watcher-mixin, CLI notification, and Telegram chunking targets
  • Canonical WSL runner: 188 passed across the six related test files
  • ruff check on all changed Python files: passed
  • py_compile on all changed Python files: passed
  • git diff --check: passed
  • Independent pre-commit review: passed; no logic or security findings
  • Full canonical sweep reached 29% with 12,715 passing tests before being stopped after unrelated Discord timing failures. An isolated retry made test_discord_connect.py green; test_discord_liveness.py retains one pre-existing ack_stale versus latency_non_finite race. Neither that test nor Discord production code is changed by this branch. CI remains the authoritative Linux full-suite gate.

Deployment / rollback

  • Deploy commit: 00d29b89a725c4a3499d0532636d86a0bc867465
  • Runtime file: gateway/kanban_watchers.py
  • Regression files: tests/gateway/test_kanban_notifier.py, tests/gateway/test_kanban_notifier_watcher_dispatch_gate.py, tests/hermes_cli/test_kanban_notify.py
  • Restart required after install: hermes gateway restart
  • Keep kanban.dispatch_in_gateway: false. This PR does not change gateway config; regression coverage proves notifier delivery with both the canonical config gate and legacy env gate false while dispatch_once remains uncalled.
  • Rollback: revert 00d29b89a725c4a3499d0532636d86a0bc867465 (restoring prior commit e361c5e20), reinstall/redeploy that revision, then run hermes gateway restart.

Safety

No Telegram calls, live board status mutations, gateway config changes, extra daemons, cron dispatchers, or alternate claim/promote/spawn paths are introduced. The parent task owns the synthetic subscribed end-to-end test after review/deployment.

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery comp/cron Cron scheduler and job management P3 Low — cosmetic, nice to have needs-decision Awaiting maintainer decision before any implementation sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jul 19, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for isolating the notifier-only use case and adding coverage for disabled embedded dispatch.

Problems

  • gateway/kanban_watchers.py:134-157 removes a deliberate multi-gateway guard. The current contract in docs/kanban/multi-gateway.md:13-17 is that the shared gate ensures exactly one process opens board DBs, avoiding WAL -shm reader contention. This patch needs an independent notifier ownership/lease mechanism before all non-dispatch gateways can poll.
  • The changed configuration semantics are not reflected in docs/kanban/multi-gateway.md:9-38; website/docs/user-guide/features/kanban.md:863 also omits the new scheduled notification event.
  • GitHub reports the branch conflicting with main, and current main includes notifier-path commits after its base (246eacea7b, 2cc0ff44b6), so the patch needs reconciliation with those paths.

Suggested changes

  • Coordinate notifier ownership per board independently of dispatcher ownership, with a multi-gateway regression proving non-owners do not open/poll the shared DB.
  • Update the affected Kanban deployment and notification documentation.

Automated hermes-sweeper review.

purely a fan-out of the single-DB poll.

Notification delivery is intentionally independent from embedded
dispatch. A gateway may own the user's chat subscription while task

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This decouples notification delivery by removing the only multi-gateway DB-access guard. docs/kanban/multi-gateway.md:13-17 defines that guard as the protection against multiple processes opening the same board DB and amplifying WAL -shm contention. Please add notifier-specific per-board ownership/coordination rather than allowing every non-dispatch gateway to poll.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 25, 2026

@DeliciousHouse DeliciousHouse left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dev-team merge-gate review at exact head 00d29b89a725c4a3499d0532636d86a0bc867465.

Verdict: changes required before merge (first and only correction cycle).

  1. HIGH (confidence 10/10) — independent notifier ownership is missing. gateway/run.py:7925-7928 starts the notifier in every gateway process, and this diff removes the only early ownership gate at gateway/kanban_watchers.py:134-157. Every non-dispatch gateway would therefore enumerate and open every board DB every five seconds. That contradicts the existing multi-gateway contract in docs/kanban/multi-gateway.md:13-17 and reintroduces the WAL/open-handle contention that gate prevents. Add notifier ownership/lease coordination independent of dispatcher ownership. Prove the selected notifier gateway delivers with dispatch_in_gateway=false, a non-owner does not open/poll board DBs, both dev boards are seen, and dispatch_once is never called.

  2. HIGH (confidence 10/10) — reconcile the live conflict without regressing current notifier delivery. Current upstream/main is 07e97d2f5dc3d2092cfe693ef07b2527a36cd2d8; the PR is merge-dirty because gateway/kanban_watchers.py overlaps 246eacea7 and 2cc0ff44b. Preserve upstream adapter_supports_push, SendResult(success=False) handling, API-server self-post-before-cursor-advance, failed-wake rewind/drop behavior, and their tests. Integrate this PR's full blocked/scheduled reason into that path: once scheduled is globally notifiable, the non-push path must not silently claim it with no self-post, and blocked/scheduled wake delivery must retain ASK, WHY GATED, SCOPE, ROLLBACK, REPLY, plus scheduled WINDOW. Add API-server regressions for successful advance, failed-wake rewind, complete decision fields, and final completion unsubscribe.

  3. MEDIUM (confidence 10/10) — update the affected docs. docs/kanban/multi-gateway.md:9-39 still says notifier and dispatcher share one ownership gate, and website/docs/user-guide/features/kanban.md:861-875 omits scheduled. Document the corrected notifier-owner model and complete scheduled notification behavior.

Evidence on this stale exact head: 43 focused Windows tests passed; 237 related canonical WSL tests passed; Ruff, py_compile, and git diff --check passed. The production diff adds no dispatcher/claim/promote/spawn path, credential, live Telegram/config mutation, or live-board mutation. These results do not clear the merge gate: the corrected exact head still needs green upstream CI, and the live PR currently reports no checks.

@DeliciousHouse DeliciousHouse left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up review of corrected head cab86e38a17afc53e6730a47d1306f6a170f2c55 against current upstream/main 0e2808729e20e09dfe3581b15ab1018f839ff636: not ready. The single developer correction cycle has already been used, so I am not issuing a second bounce; disposition is escalated to dev-lead.

Verified blockers:

  1. [P1] The machine-global lease starves disjoint single-profile gateways. Only the lease winner polls (gateway/kanban_watchers.py:144-180). In the supported one-process-per-profile topology, that process has no adapters for the other profiles; gateway/kanban_watchers.py:319-327 skips those subscriptions while every other gateway is locked out. A default winner starves named-profile subscriptions, and a named-profile winner starves default/other-profile subscriptions. Startup order cannot make simultaneous subscriptions for both processes deliver.

  2. [P1] An active named-profile gateway cannot route even its own stamped subscription. The named profile's primary adapter is in self.adapters, but notifier delivery passes the name to _authorization_adapter (gateway/kanban_watchers.py:377-400), whose non-default branch consults only _profile_adapters and returns None (gateway/authz_mixin.py:64-89). An isolated two-tick reproduction with active profile beta, adapter in self.adapters, and a completed beta subscription produced sends=0 and left completed unseen.

  3. [P1] Multiplex auto-subscription stamps the wrong profile. /kanban create reads the inbound source but ignores source.profile, stamping the gateway's startup notifier profile instead (gateway/slash_commands.py:475-493). An isolated reproduction produced source_profile=beta and stamped_notifier_profile=default, so later delivery can use the wrong same-platform bot.

  4. [P1] Partial batch failure can duplicate an already delivered decision/scheduled event indefinitely. Claimed events are sent individually, each success clears failure_count, and a later failure rewinds the whole batch (gateway/kanban_watchers.py:562-592). The next tick re-sends the earlier event and clears the counter before the same later failure, so retry exhaustion and terminal unsubscribe may never occur. This violates the no-double-send/cursor/final-unsubscribe acceptance contract.

  5. [P2] Adapter loss does not release notifier ownership. Adapter presence is checked before election, but an elected owner with no active platforms only sleeps and retains the lease until its owner loop exits (gateway/kanban_watchers.py:131-180,269-278). A healthy waiting gateway therefore cannot take over during a prolonged adapter outage.

Verification evidence:

  • Exact head unchanged; draft; no CI checks reported, so the green-CI merge precondition is also unmet.
  • Current-main merge tree is clean; branch and merge-effect git diff --check pass.
  • Focused Windows notifier/mixin/CLI/Telegram files: 47 passed. The broader Windows platform-base file has five pre-existing path-semantics failures (188 passed, 2 skipped), matching prior evidence.
  • Canonical WSL run across seven relevant files: 242 passed.
  • Ruff and Python compilation: pass.
  • Independent read-only Codex review: BLOCK, independently confirming the lease/profile-routing and retry-ordering failures.

No code, config, live messaging, or board-runtime mutation was performed by the review.

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

Labels

comp/cron Cron scheduler and job management comp/gateway Gateway runner, session dispatch, delivery needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants