Skip to content

fix(notifications): event-driven delivery via _emit_event callback - #36132

Open
someaka wants to merge 5 commits into
NousResearch:mainfrom
someaka:fix/notification-event-driven
Open

fix(notifications): event-driven delivery via _emit_event callback#36132
someaka wants to merge 5 commits into
NousResearch:mainfrom
someaka:fix/notification-event-driven

Conversation

@someaka

@someaka someaka commented May 31, 2026

Copy link
Copy Markdown

What

Replace direct completion_queue.put() calls in ProcessRegistry with _emit_event() which fires both the queue AND a registered _completion_callback. This is the core plumbing that enables event-driven notification delivery.

Why

The notification pipeline has three layers:

  1. This PRprocess_registry.py: _emit_event() fires callback + queue (the core fix)
  2. fix(notifications): event-driven TUI adapter, interrupt wait, queue drain #36089tui_adapter.py + TUI client fixes: HTTP-based adapter that receives events
  3. fix(gateway): SSE delivery for background process notifications + subagent protection #35553gateway/run.py: SSE delivery + subagent protection

Without this PR, the TUI adapter (#36089) registers a _completion_callback but it never fires because completion_queue.put() is called directly. This PR makes the callback fire on every event.

Changes

  • Add _completion_callback attribute to ProcessRegistry.__init__
  • Add _emit_event() method: queue.put() + fire callback
  • Replace all 5 completion_queue.put() calls with _emit_event()
  • Add mark_completion_consumed() for explicit consumption tracking
  • Remove poll() auto-consumption (poll is read-only, mark_completion_consumed() is the explicit writer)

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/tui Terminal UI (ui-tui/ + tui_gateway/) comp/tools Tool registry, model_tools, toolsets comp/gateway Gateway runner, session dispatch, delivery labels Jun 1, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Related to #21597 (event-driven approach, still open) and #36089 (same author, broader TUI adapter fix). This PR provides the core ProcessRegistry plumbing that #36089 depends on. See also merged #26327 (polling-drain approach).

@someaka
someaka force-pushed the fix/notification-event-driven branch 3 times, most recently from fe28f47 to 4a42dbc Compare June 8, 2026 22:22
Radical Edward added 5 commits June 9, 2026 02:57
Replace direct completion_queue.put() calls with _emit_event() which
fires both the queue AND the registered _completion_callback. This is
the core fix that enables event-driven notification delivery to TUI/CLI
sessions without polling.

Changes:
- Add _completion_callback attribute to ProcessRegistry.__init__
- Add _emit_event() method: queue.put + fire callback
- Replace all completion_queue.put() calls with _emit_event()
- Add mark_completion_consumed() for explicit consumption tracking
- Remove poll() auto-consumption (poll is read-only)
- Add Platform.TUI and Platform.CLI to gateway/config.py enum
- Add connected checkers (always True when running)
- Add regression test: poll() must NOT mark completions consumed
…peline

- test_notification_e2e.py: full pipeline test (create → subscribe →
  complete → verify event → verify delivery → cursor advancement)
- test_regression_notification_paths.py: regression guards for TUI
  poller, gateway watcher, CLI drain, completion-consumed semantics
- test_fifo_notification_bridge.py: DB-based notification bridge tests
  (writer: kanban_db._append_event, reader: tui_gateway/server.py)
@someaka
someaka force-pushed the fix/notification-event-driven branch from 4a42dbc to 9d1d7f4 Compare June 9, 2026 00:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery comp/tools Tool registry, model_tools, toolsets comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants