fix(cron): carry Slack workspace scope_id into continuable seed keys
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:<chat>:<thread> while a real scoped reply keys agent:main:slack:dm:<team>:<chat>:<thread> — 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.
This commit is contained in:
@@ -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(
|
||||
|
||||
@@ -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:<chat>:<thread> while the real reply keys
|
||||
agent:main:slack:dm:<team>:<chat>:<thread> — 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"
|
||||
)
|
||||
|
||||
@@ -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:<chat>:<thread> while the
|
||||
# reply keys agent:main:slack:dm:<team>:<chat>:<thread>. 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
|
||||
|
||||
|
||||
Reference in New Issue
Block a user