Skip to content

fix(tui): poll kanban_notify_subs for task event delivery to TUI sessions - #59963

Closed
isheng-eqi wants to merge 6 commits into
NousResearch:mainfrom
isheng-eqi:fix/59960-tui-kanban-notify
Closed

fix(tui): poll kanban_notify_subs for task event delivery to TUI sessions#59963
isheng-eqi wants to merge 6 commits into
NousResearch:mainfrom
isheng-eqi:fix/59960-tui-kanban-notify

Conversation

@isheng-eqi

Copy link
Copy Markdown
Contributor

Summary

Fixes #59960

Problem

Kanban task event notifications are never delivered to TUI (Desktop/TUI) sessions. The root cause is a delivery-path gap:

  1. The gateway's _kanban_notifier_watcher reads kanban_notify_subs correctly but has no adapter for tui platform subscriptions — TUI is not a gateway messaging channel
  2. The TUI's _notification_poller_loop only watches process_registry.completion_queue for background process completions, never reading kanban_notify_subs

Result: kanban_tools._maybe_auto_subscribe() correctly creates subscriptions with platform="tui", but no code path ever delivers them. All TUI kanban subscriptions have last_event_id=0 forever.

Fix

Add _poll_kanban_task_events() — a new helper that mirrors the gateway watcher's subscription-polling pattern:

  • Lists kanban_notify_subs for the TUI session (filtered by platform="tui" + chat_id=HERMES_SESSION_KEY)
  • Claims unseen terminal events via kanban_db.claim_unseen_events_for_sub()
  • Emits status.update messages with kind="kanban" to the TUI session

Polled every ~5 seconds on the existing completion_queue.get(timeout=0.5) path — no additional thread or timer needed.

Changes

  • tui_gateway/server.py: add _poll_kanban_task_events() helper + integrate into _notification_poller_loop timeout path

…usResearch#58774)

_restore_or_build_system_prompt unconditionally restored the session-DB
stored prompt when it matched the current runtime identity, even when
the caller set an explicit ephemeral_system_prompt (e.g. /personality).

Check ephemeral_system_prompt before the stored-prompt fast path so a
deliberate personality switch takes effect immediately instead of being
silently ignored until the next fresh session.
…ibuteError (NousResearch#59845)

The Copilot x-initiator injection block calls agent._is_copilot_url()
without a getattr guard, unlike the sibling _is_user_initiated_turn
check one line above. On some agent construction paths (module-reload,
wrapper agents) _is_copilot_url may be missing, causing every API call
in the conversation to fail with AttributeError and the cron job to
error out.

Wrap the call with getattr(agent, '_is_copilot_url', lambda: False)()
so non-Copilot and partially-initialized agents fall through cleanly.

Github-Issue:NousResearch#59845
…nt delivery

The TUI notification poller (_notification_poller_loop) only watched
process_registry.completion_queue, never polling kanban_notify_subs.
Kanban task subscriptions with platform='tui' were therefore never
delivered — the gateway's _kanban_notifier_watcher has no TUI adapter,
and the TUI poller had no kanban polling logic.

Add _poll_kanban_task_events() which mirrors the gateway watcher's
pattern: list kanban_notify_subs for the session, claim unseen terminal
events via kanban_db.claim_unseen_events_for_sub(), and emit
status.update messages to the TUI session. Polled every ~5 seconds
on the existing completion_queue.get() timeout path.

Github-Issue:NousResearch#59960
@alt-glitch alt-glitch added type/bug Something isn't working comp/cron Cron scheduler and job management comp/tui Terminal UI (ui-tui/ + tui_gateway/) P3 Low — cosmetic, nice to have labels Jul 7, 2026

@falkoro falkoro 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.

Reviewed this while writing up the comparison with my duplicate (#60085 — happy for whichever version the maintainers prefer to land). One functional issue worth fixing either way:

_poll_kanban_task_events reads the session key from the wrong place. It uses os.environ.get("HERMES_SESSION_KEY") in the poller thread, but subscriptions are written with chat_id from the session-context bridge: kanban_tools._maybe_auto_subscribe reads get_session_env("HERMES_SESSION_KEY"), which is a ContextVar set per agent turn via _set_session_context(session["session_key"]) (gateway/session_context.py — the env fallback only applies when the ContextVar was never set). The TUI server's own process env normally has no HERMES_SESSION_KEY, so the poller returns early and never delivers; and in a multi-session desktop, a single process-level env value can't distinguish sessions — whichever value happens to be set would route every session's notifications to one key.

Since the poller already receives the session dict, the fix is one line: session_key = str(session.get("session_key") or "").

Two smaller parity notes vs the gateway notifier (gateway/kanban_watchers.py), take or leave: it iterates all boards (subs on non-default boards deliver too), and it unsubscribes at a final task status (done/archived) so rows don't accumulate per completed task.

@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 tracing the missing TUI delivery leg. The current main poller still only drains process_registry.completion_queue (tui_gateway/server.py:8719-8740), so the underlying bug remains.

Problems

  • tui_gateway/server.py:8318 reads HERMES_SESSION_KEY from os.environ. TUI turns bind the key through _set_session_context() / set_session_vars() (tui_gateway/server.py:1990-2015, 8977-8980), which uses ContextVars. The poller can therefore return early, and a process-global value would not isolate desktop sessions.
  • tui_gateway/server.py:8326 opens only the default board. The gateway notifier enumerates all boards (gateway/kanban_watchers.py:209-295), so subscriptions on an explicitly selected board would remain undelivered.
  • The change has no TUI kanban regression tests, and it retains final subscriptions unlike the gateway cleanup rule (gateway/kanban_watchers.py:484-492).

Suggested changes

  • Key the poller from session["session_key"], then test cross-session isolation.
  • Mirror gateway multi-board polling and final-status unsubscribe behavior.
  • Keep the salvage focused on the TUI change; the unrelated core prompt/cache changes should be separated.

This is an automated hermes-sweeper review.

Comment thread tui_gateway/server.py
from hermes_cli import kanban_db as _kb

session_key = os.environ.get("HERMES_SESSION_KEY", "")
if not session_key:

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.

Use str(session.get("session_key") or "") here. TUI turns bind this key through the session-context ContextVar, not the server process environment; this poller otherwise normally returns early and a shared env value cannot isolate simultaneous desktop sessions.

Comment thread tui_gateway/server.py
try:
conn = _kb.connect()
except Exception:
return

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 only opens the default board, but subscriptions are stored in the board selected when kanban_create connects. Mirror the gateway notifier's all-board iteration so non-default-board TUI subscriptions can be delivered.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 15, 2026
@falkoro

falkoro commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Nice tracing of the delivery-path gap — the two-leg diagnosis (no TUI adapter in the gateway watcher, poller only draining completion_queue) matches what I found on this bug too.

Coordination note: the earlier #60085 (open, fixes #59890 — which looks like the same underlying report as #59960) implements this same missing leg, and it already covers the three items in the sweeper review, verifiable in the diff:

  • Session keying — reads session["session_key"] (per-session dict, ContextVar-fed), not os.environ; includes a test_no_session_key_is_a_noop guard for the early-return case and cross-session isolation.
  • Multi-board — enumerates list_boards(include_archived=False) with a default-board fallback, deduping by resolved DB path so aliased slugs aren't double-polled (mirrors gateway/kanban_watchers.py's all-boards behavior).
  • Final-status unsubscribe — removes the subscription only at truly final statuses (done/archived), matching the gateway cleanup rule, with a regression test for the blocked-stays-subscribed case.

8 TUI kanban regression tests total in tests/tui_gateway/test_kanban_notify_poller.py. Happy to consolidate in either direction — if there's anything in this PR's approach the maintainers prefer (e.g. the auto-subscribe wiring), I'm glad to fold it into #60085 with credit, or review an updated version of this one against the same test set.

@teknium1

Copy link
Copy Markdown
Contributor

Fixed on main via PR #72177 — a salvage of #66435, the current-main cherry-pick of @falkoro's #60085, which implemented the same missing TUI consumer this PR targeted. #60085 was submitted five days before this one and was ultimately the branch taken, with its authorship preserved in git log (badb240, 6247712).

Thanks for the independent root-cause and the focused implementation here — the diagnosis (gateway watcher has no "tui" adapter, TUI poller only drains the process completion queue) matched exactly. Closing since the delivery path is now on main; if you spot gaps in the landed version, a fresh PR on top is very welcome.

@teknium1 teknium1 closed this Jul 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/cron Cron scheduler and job management comp/tui Terminal UI (ui-tui/ + tui_gateway/) P3 Low — cosmetic, nice to have sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] TUI kanban notifications never delivered — poller only watches process completions, not kanban_notify_subs

4 participants