From 8b6cf434cbbd97b228afbc499281b10cbd56746f Mon Sep 17 00:00:00 2001 From: Ben Barclay Date: Thu, 20 Aug 2026 20:04:49 +1000 Subject: [PATCH] fix(cron): carry Slack workspace scope_id into continuable seed keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit build_session_key embeds the workspace segment (scope_id) in every Slack dm/group/thread key, but both cron seed helpers built their SessionSource without it: the seeded row keyed agent:main:slack:dm:: while a real scoped reply keys agent:main:slack:dm::: — a row no reply ever resolves to. DMs were rescued only incidentally by the legacy-key claim-once migration; scoped channels/threads got continuation amnesia, and identical channel ids in two workspaces could collide. Capture HERMES_SESSION_SCOPE_ID into the cron origin (_origin_from_env — the session-context var async_delegation already snapshots), add scope_id to _seed_cron_thread_session/_seed_cron_channel_session, and pass the origin's scope at all three seed call sites. Tests: scoped dm-thread / channel-thread / flat-channel seed-vs-reply key equality through the real build_session_key, plus a two-workspace non-collision guard. --- cron/scheduler.py | 16 +++ tests/cron/test_cron_thread_seed_dm_keying.py | 108 +++++++++++++++++- tools/cronjob_tools.py | 10 ++ 3 files changed, 133 insertions(+), 1 deletion(-) diff --git a/cron/scheduler.py b/cron/scheduler.py index 7e9304b2d5..fb4926c4c5 100644 --- a/cron/scheduler.py +++ b/cron/scheduler.py @@ -1731,6 +1731,7 @@ def _seed_cron_thread_session( mirror_text: str, chat_name: Optional[str] = None, is_dm: bool = False, + scope_id: Optional[str] = None, ) -> None: """Seed the freshly-opened cron thread's session with the brief. @@ -1741,6 +1742,12 @@ def _seed_cron_thread_session( threads as participant-shared, so no ``user_id`` is needed) and append the brief as an assistant turn via the shipped ``mirror_to_session``. + ``scope_id`` is the workspace/server scope (Slack team id). + ``build_session_key`` embeds it in every Slack key, so a scoped reply's + key carries it — the seed must reproduce it or the seeded row is + unreachable (the scope-less flat-seed sibling of the is_dm keying bug). + Best-effort None for platforms without scope. + ``is_dm`` selects the seeded ``chat_type``: a thread under a DM must seed ``chat_type="dm"`` because the user's in-thread DM reply arrives with chat_type="dm" and ``build_session_key`` routes DM threads through the DM @@ -1791,6 +1798,7 @@ def _seed_cron_thread_session( user_id="system:cron", user_name="Cron", thread_id=str(thread_id), + scope_id=str(scope_id) if scope_id else None, ) # Ensure the thread-keyed session row exists so the mirror has # a target and the user's later reply joins the same session. @@ -1847,6 +1855,7 @@ def _seed_cron_channel_session( is_dm: bool, user_id: Optional[str], chat_name: Optional[str] = None, + scope_id: Optional[str] = None, ) -> bool: """Seed the FLAT (thread_id=None) session for an ``in_channel`` cron delivery. @@ -1904,6 +1913,10 @@ def _seed_cron_channel_session( chat_type=chat_type, user_id=str(user_id) if user_id else None, thread_id=None, # flat — the whole-channel/DM session + # Workspace scope: build_session_key embeds it in every + # Slack key, so a scoped reply only resolves to this row + # when the seed carries it too (see thread-seed docstring). + scope_id=str(scope_id) if scope_id else None, ) # Create the flat session row so the mirror has a target and the # user's later plain reply joins the SAME session. Capture the @@ -3144,6 +3157,7 @@ def _deliver_result(job: dict, content: str, adapters=None, loop=None) -> Option opened_thread_id, mirror_text, chat_name=origin.get("chat_name"), is_dm=is_dm_target, + scope_id=origin.get("scope_id"), ) thread_seeded = True # in_channel surface: CREATE + seed the flat channel/DM @@ -3163,6 +3177,7 @@ def _deliver_result(job: dict, content: str, adapters=None, loop=None) -> Option mirror_text, is_dm=is_dm_target, user_id=origin_user_id, chat_name=origin.get("chat_name"), + scope_id=origin.get("scope_id"), ) if not inchannel_seeded: logger.warning( @@ -3184,6 +3199,7 @@ def _deliver_result(job: dict, content: str, adapters=None, loop=None) -> Option str(delivered_message_id), mirror_text, chat_name=origin.get("chat_name"), is_dm=is_dm_target, + scope_id=origin.get("scope_id"), ) elif in_channel_surface and not origin_target: logger.warning( diff --git a/tests/cron/test_cron_thread_seed_dm_keying.py b/tests/cron/test_cron_thread_seed_dm_keying.py index 83194c2d16..e6a738f4aa 100644 --- a/tests/cron/test_cron_thread_seed_dm_keying.py +++ b/tests/cron/test_cron_thread_seed_dm_keying.py @@ -16,7 +16,7 @@ not on SessionSource field shapes — pins the end-to-end contract. from unittest.mock import MagicMock, patch -from cron.scheduler import _seed_cron_thread_session +from cron.scheduler import _seed_cron_channel_session, _seed_cron_thread_session from gateway.config import Platform from gateway.session import SessionSource, build_session_key @@ -95,3 +95,109 @@ def test_dm_seed_default_is_backward_compatible(): ) assert _seeded_source(store).chat_type == "thread" + + +def test_scoped_dm_thread_seed_key_matches_scoped_reply_key(): + """Slack keys embed the workspace scope_id (build_session_key puts the + team segment in every Slack dm/group/thread key). A seed built without + it creates agent:main:slack:dm:: while the real reply keys + agent:main:slack:dm::: — a row no scoped reply ever + resolves to. The seed must carry the origin's scope_id.""" + store = MagicMock() + adapter = MagicMock() + adapter._session_store = store + + with patch("gateway.mirror.mirror_to_session", return_value=True): + _seed_cron_thread_session( + {"id": "j4", "name": "digest"}, adapter, "slack", + "D0BJTDCSR7C", "1787188136.448949", "Three bullets", + chat_name=None, is_dm=True, scope_id="T0AAAA111", + ) + + reply_source = SessionSource( + platform=Platform.SLACK, + chat_id="D0BJTDCSR7C", + chat_type="dm", + user_id="U0B5F8EEYAD", + thread_id="1787188136.448949", + scope_id="T0AAAA111", + ) + assert build_session_key(_seeded_source(store)) == build_session_key( + reply_source + ), ( + "seeded key lacks the workspace scope segment — a scoped Slack " + "reply resolves to a different row (continuation amnesia)" + ) + + +def test_scoped_channel_thread_seed_key_matches_scoped_reply_key(): + store = MagicMock() + adapter = MagicMock() + adapter._session_store = store + + with patch("gateway.mirror.mirror_to_session", return_value=True): + _seed_cron_thread_session( + {"id": "j5", "name": "digest"}, adapter, "slack", + "C0AAAAAAAA", "1787188000.000100", "Three bullets", + chat_name="ops", is_dm=False, scope_id="T0AAAA111", + ) + + reply_source = SessionSource( + platform=Platform.SLACK, + chat_id="C0AAAAAAAA", + chat_type="thread", + user_id="U0B5F8EEYAD", + thread_id="1787188000.000100", + scope_id="T0AAAA111", + ) + assert build_session_key(_seeded_source(store)) == build_session_key( + reply_source + ) + + +def test_scoped_flat_channel_seed_key_matches_scoped_reply_key(): + """The flat in_channel seed must reproduce the scoped key too.""" + store = MagicMock() + adapter = MagicMock() + adapter._session_store = store + + with patch("gateway.mirror.mirror_to_session", return_value=True): + _seed_cron_channel_session( + {"id": "j6", "name": "digest"}, adapter, "slack", + "C0AAAAAAAA", "Three bullets", is_dm=False, + user_id="U0B5F8EEYAD", chat_name="ops", scope_id="T0AAAA111", + ) + + reply_source = SessionSource( + platform=Platform.SLACK, + chat_id="C0AAAAAAAA", + chat_type="group", + user_id="U0B5F8EEYAD", + thread_id=None, + scope_id="T0AAAA111", + ) + assert build_session_key(_seeded_source(store)) == build_session_key( + reply_source + ) + + +def test_seeds_do_not_collide_across_workspaces(): + """Two workspaces sharing a Slack chat id must seed DISTINCT keys — + the exact cross-tenant collision the workspace key segment exists to + prevent.""" + keys = [] + for team in ("T0AAAA111", "T0BBBB222"): + store = MagicMock() + adapter = MagicMock() + adapter._session_store = store + with patch("gateway.mirror.mirror_to_session", return_value=True): + _seed_cron_channel_session( + {"id": f"j-{team}"}, adapter, "slack", + "C0AAAAAAAA", "brief", is_dm=False, + user_id="U0B5F8EEYAD", scope_id=team, + ) + keys.append(build_session_key(_seeded_source(store))) + assert keys[0] != keys[1], ( + "identical chat ids in different workspaces seeded the SAME session " + "key — cross-workspace transcript bleed" + ) diff --git a/tools/cronjob_tools.py b/tools/cronjob_tools.py index 502dc4a74e..1130174015 100644 --- a/tools/cronjob_tools.py +++ b/tools/cronjob_tools.py @@ -352,6 +352,16 @@ def _origin_from_env() -> Optional[Dict[str, str]]: # send_message, which passes HERMES_SESSION_USER_ID to # gateway.mirror.mirror_to_session. Harmless for DMs/shared sessions. "user_id": get_session_env("HERMES_SESSION_USER_ID") or None, + # Workspace/server scope (Slack team, Discord guild, Matrix + # server). build_session_key embeds it in every Slack session key + # (dm/group/thread alike), so a continuable cron seed built + # WITHOUT it creates a row no scoped reply ever resolves to — + # the seeded key is agent:main:slack:dm:: while the + # reply keys agent:main:slack:dm:::. Captured + # here so the scheduler's seed helpers can reproduce the reply's + # exact key. Same session-context var async_delegation already + # snapshots; None for platforms without scope. + "scope_id": get_session_env("HERMES_SESSION_SCOPE_ID") or None, } return None