fix(kanban): inherit ALL routing columns in notify-sub inheritance
_inherit_notify_subs (link_tasks / triage-decompose / create-parents path) copied only platform/chat/thread/user/profile, dropping chat_type, user_id_alt, delivery_mode, and delivery_metadata. A DM-originated child completion then fell back to chat_type='group' and woke a fresh group-scoped session instead of the originating DM; Telegram DM-topic subs lost their persisted reply-fallback metadata (issue #73030). Consolidates the duplicated inline inheritance block in create_task onto the single-owner helper — one inheritance path, every column, ONE owner. Sabotage-verified regression tests for both the link_tasks and create-with-parents paths.
This commit is contained in:
parent
8855766716
commit
c495be19aa
|
|
@ -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})
|
||||
""",
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
Loading…
Reference in New Issue