diff --git a/hermes_cli/kanban_db.py b/hermes_cli/kanban_db.py index e92bc543a8fe6..d770de2a838ce 100644 --- a/hermes_cli/kanban_db.py +++ b/hermes_cli/kanban_db.py @@ -3494,47 +3494,10 @@ def create_task( "INSERT OR IGNORE INTO task_links (parent_id, child_id) VALUES (?, ?)", (pid, task_id), ) - if parents: - # ACK-edge inheritance: a child inherits the parent/root - # task's terminal-notification return path (chat_type and - # delivery_mode included), so the originating channel still - # hears about a child that BLOCKs, not just the final fan-in. - # The child task_id is brand-new, so INSERT OR IGNORE copies - # each sub. - placeholders = ",".join("?" * len(parents)) - parent_subs = conn.execute( - "SELECT * FROM kanban_notify_subs " - f"WHERE task_id IN ({placeholders}) " - "ORDER BY created_at ASC", - parents, - ).fetchall() - for psub in parent_subs: - # Inherit chat_type and delivery_mode so a woken child - # notification keys to the parent's channel. - psub_mode = psub["delivery_mode"] or "notify" - psub_chat_type = psub["chat_type"] or "dm" - conn.execute( - """ - INSERT OR IGNORE INTO kanban_notify_subs - (task_id, platform, chat_id, thread_id, user_id, user_id_alt, - chat_type, notifier_profile, delivery_mode, - delivery_metadata, created_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) - """, - ( - task_id, - psub["platform"], - psub["chat_id"], - psub["thread_id"] or "", - psub["user_id"], - psub["user_id_alt"], - psub_chat_type, - psub["notifier_profile"], - psub_mode, - psub["delivery_metadata"], - now, - ), - ) + # Notify-sub inheritance (ACK-edge: the originating channel + # still hears about a child that BLOCKs, not just the final + # fan-in) is handled by the single-owner helper below — + # _inherit_notify_subs copies every routing/delivery column. _append_event( conn, task_id, @@ -3590,6 +3553,13 @@ def _inherit_notify_subs( cursor. This makes manual `link_tasks(parent, existing_child)` safe: the parent chat receives future child terminal events without replaying the child's pre-link history. + + Copies EVERY routing/delivery column (chat_type, user_id_alt, + delivery_mode, delivery_metadata included) — this helper is the single + owner of subscription inheritance for create_task, link_tasks, and triage + decomposition. Omitting columns here silently degrades routing: a + DM-originated child completion falls back to chat_type='group' and wakes + a fresh group-scoped session instead of the originating DM (issue #73030). """ parent_ids = tuple(dict.fromkeys(p for p in parents if p)) if not parent_ids: @@ -3603,9 +3573,12 @@ def _inherit_notify_subs( conn.execute( f""" INSERT OR IGNORE INTO kanban_notify_subs - (task_id, platform, chat_id, thread_id, user_id, - notifier_profile, created_at, last_event_id) - SELECT ?, platform, chat_id, thread_id, user_id, notifier_profile, ?, ? + (task_id, platform, chat_id, thread_id, user_id, user_id_alt, + chat_type, notifier_profile, delivery_mode, delivery_metadata, + created_at, last_event_id) + SELECT ?, platform, chat_id, thread_id, user_id, user_id_alt, + COALESCE(chat_type, 'dm'), notifier_profile, + COALESCE(delivery_mode, 'notify'), delivery_metadata, ?, ? FROM kanban_notify_subs WHERE task_id IN ({placeholders}) """, diff --git a/tests/hermes_cli/test_kanban_notify.py b/tests/hermes_cli/test_kanban_notify.py index b104d0f9eab79..28299e3e5fba1 100644 --- a/tests/hermes_cli/test_kanban_notify.py +++ b/tests/hermes_cli/test_kanban_notify.py @@ -939,3 +939,75 @@ def test_migration_backfill_runs_only_on_first_add(kanban_home): (task_id,), ).fetchone() assert row["delivery_mode"] == "notify" + + +# --------------------------------------------------------------------------- +# Issue #73030: _inherit_notify_subs (link_tasks / decompose path) must copy +# EVERY routing column — chat_type, user_id_alt, delivery_mode, and +# delivery_metadata. Before the fix it copied only platform/chat/thread/user/ +# profile, so a DM-originated child completion fell back to chat_type='group' +# and woke a fresh group-scoped session instead of the originating DM, and +# Telegram DM-topic subs lost their persisted reply-fallback metadata. +# --------------------------------------------------------------------------- + + +def _add_full_parent_sub(kb, conn, parent): + kb.add_notify_sub( + conn, task_id=parent, platform="telegram", chat_id="chat1", + thread_id="topic1", user_id="user1", user_id_alt="alt-1", + chat_type="dm", notifier_profile="default", + delivery_mode="notify+wake", + delivery_metadata={"reply_fallback": "general", "topic_name": "ops"}, + ) + + +def _assert_full_inherited_sub(subs): + assert len(subs) == 1 + s = subs[0] + assert s["platform"] == "telegram" + assert s["chat_id"] == "chat1" + assert s["thread_id"] == "topic1" + assert s["user_id"] == "user1" + assert s["user_id_alt"] == "alt-1", "user_id_alt dropped during inheritance" + assert s["chat_type"] == "dm", ( + "chat_type dropped during inheritance — wake would key to a " + "group-scoped session instead of the originating DM (issue #73030)" + ) + assert s["delivery_mode"] == "notify+wake" + md = s["delivery_metadata"] + assert md and md.get("reply_fallback") == "general", ( + "delivery_metadata dropped during inheritance (issue #73030)" + ) + + +def test_link_tasks_inherits_all_routing_columns(kanban_home): + import hermes_cli.kanban_db as kb + + conn = kb.connect() + try: + parent = kb.create_task(conn, title="root", assignee=None) + _add_full_parent_sub(kb, conn, parent) + # Pre-existing child, linked after the fact — exercises + # _inherit_notify_subs directly (not the create_task parents path). + child = kb.create_task(conn, title="existing child", assignee="w1") + kb.link_tasks(conn, parent, child) + subs = kb.list_notify_subs(conn, child) + finally: + conn.close() + _assert_full_inherited_sub(subs) + + +def test_create_with_parents_inherits_delivery_metadata(kanban_home): + import hermes_cli.kanban_db as kb + + conn = kb.connect() + try: + parent = kb.create_task(conn, title="root", assignee=None) + _add_full_parent_sub(kb, conn, parent) + child = kb.create_task( + conn, title="graph child", assignee="w1", parents=[parent], + ) + subs = kb.list_notify_subs(conn, child) + finally: + conn.close() + _assert_full_inherited_sub(subs)