fix(cron): in_channel seed must not require the attach_to_session mirror opt-in
Live regression (Alice, 2026-08-19): a continuable cron with cron_continuable_surface=in_channel delivered its brief flat, but the flat-session seed was gated on mirror_this_target = mirror_enabled AND origin-match. Without attach_to_session (and with cron.mirror_delivery defaulting False) the seed never ran; the next plain reply resolved to a blank (slack, chat, None) session and the agent had no idea about its own delivery message — not continuable in channel OR thread (in_channel mode correctly skips thread creation, so there was no thread session either). in_channel IS the continuation surface, not a mirror nicety: gate the seed on origin-match alone, and resolve origin_user_id for any origin-matching target so the seeded key still carries the scheduling user on per-user-isolated chats. attach_to_session remains the opt-in for the separate default-surface mirror behavior. Regression test drives the delivery path with attach_to_session=False and asserts the seed fires with the right user_id (fails on pre-fix code).
This commit is contained in:
+14
-5
@@ -2676,12 +2676,14 @@ def _deliver_result(job: dict, content: str, adapters=None, loop=None) -> Option
|
||||
# Mirror is scoped to the ORIGIN conversation only. A fan-out / broadcast
|
||||
# / home-channel-fallback target is never mirrored (it is not the
|
||||
# conversation the job was created in, and may have no session at all).
|
||||
mirror_this_target = mirror_enabled and _target_matches_origin(
|
||||
origin, platform_name, chat_id, thread_id
|
||||
)
|
||||
origin_target = _target_matches_origin(origin, platform_name, chat_id, thread_id)
|
||||
mirror_this_target = mirror_enabled and origin_target
|
||||
# Pass the origin's user_id so a per-user-isolated group chat resolves to
|
||||
# the exact member who scheduled the job — parity with send_message.
|
||||
origin_user_id = origin.get("user_id") if mirror_this_target else None
|
||||
# Resolved for ANY origin-matching target (not just mirror-enabled):
|
||||
# the in_channel seed below needs it too, and it must not depend on
|
||||
# the attach_to_session/mirror opt-in.
|
||||
origin_user_id = origin.get("user_id") if origin_target else None
|
||||
|
||||
# Built-in names resolve to their enum member; plugin platform names
|
||||
# create dynamic members via Platform._missing_().
|
||||
@@ -3102,7 +3104,14 @@ def _deliver_result(job: dict, content: str, adapters=None, loop=None) -> Option
|
||||
# session (the shipped mirror only appends to an existing
|
||||
# session — the flat row is otherwise absent for a
|
||||
# chat_postMessage delivery, so the brief would be lost).
|
||||
if in_channel_surface and mirror_this_target and not thread_seeded:
|
||||
# Gated on ORIGIN-match only, NOT on the mirror opt-in:
|
||||
# in_channel IS the continuation surface — a continuable
|
||||
# flat cron without its seed is a brief the next reply
|
||||
# can't see (the bug Victor hit live 2026-08-19: agent had
|
||||
# "no idea about the delivery message"). attach_to_session
|
||||
# remains the knob for the SEPARATE thread/default-surface
|
||||
# mirror behavior; it must not be required here.
|
||||
if in_channel_surface and origin_target and not thread_seeded:
|
||||
inchannel_seeded = _seed_cron_channel_session(
|
||||
job, runtime_adapter, platform_name, chat_id,
|
||||
mirror_text, is_dm=is_dm_target,
|
||||
|
||||
@@ -2337,7 +2337,8 @@ class TestCronContinuableSurfaceInChannel:
|
||||
mock_cfg.platforms = {Platform.SLACK: pconfig}
|
||||
return mock_cfg
|
||||
|
||||
def _run_inchannel_delivery(self, extra, adapter, *, mirror_ok=True, origin=None):
|
||||
def _run_inchannel_delivery(self, extra, adapter, *, mirror_ok=True, origin=None,
|
||||
attach_to_session=True):
|
||||
"""Drive _deliver_result down the live-adapter path for a Slack
|
||||
channel-origin job with the given ``extra`` config. Returns the
|
||||
_open_continuable_cron_thread mock and the mirror_to_session mock."""
|
||||
@@ -2366,8 +2367,9 @@ class TestCronContinuableSurfaceInChannel:
|
||||
# Carries the scheduling user's id — the in_channel seed must key
|
||||
# the flat channel session to THIS user (see build_session_key).
|
||||
"origin": origin or {"platform": "slack", "chat_id": "C123", "user_id": "U_HUMAN"},
|
||||
# Opt into the continuable mirror.
|
||||
"attach_to_session": True,
|
||||
# Opt into the continuable mirror (parameterized: the seed must
|
||||
# NOT depend on this — see the no-attach regression test).
|
||||
"attach_to_session": attach_to_session,
|
||||
}
|
||||
|
||||
with patch("gateway.config.load_gateway_config", return_value=mock_cfg), \
|
||||
@@ -2440,6 +2442,30 @@ class TestCronContinuableSurfaceInChannel:
|
||||
assert mirror_mock.call_args.kwargs.get("thread_id") is None
|
||||
assert mirror_mock.call_args.kwargs.get("user_id") == "U_HUMAN"
|
||||
|
||||
def test_in_channel_seed_fires_without_attach_to_session(self):
|
||||
"""REGRESSION (live, Alice 2026-08-19): the in_channel seed was gated on
|
||||
mirror_this_target (mirror_enabled AND origin match), so a continuable
|
||||
in_channel cron created WITHOUT attach_to_session (and with the
|
||||
cron.mirror_delivery global at its default False) delivered the brief
|
||||
flat but never seeded the flat session — the next plain reply hit a
|
||||
blank session and the agent had no idea about its own delivery message.
|
||||
|
||||
in_channel IS the continuation surface: the seed must fire on origin
|
||||
match alone. attach_to_session stays the opt-in for the SEPARATE
|
||||
default-surface mirror behavior; it must not be required here."""
|
||||
from cron.scheduler import _deliver_result # noqa: F401 (driven via helper)
|
||||
|
||||
adapter = self._slack_adapter(supports_inchannel=True)
|
||||
with patch("cron.scheduler._seed_cron_channel_session", return_value=True) as seed_mock:
|
||||
self._run_inchannel_delivery(
|
||||
{"slack": {"cron_continuable_surface": "in_channel"}}, adapter,
|
||||
attach_to_session=False,
|
||||
)
|
||||
seed_mock.assert_called_once()
|
||||
# user_id must ride along even without the mirror opt-in — the flat
|
||||
# session key includes it on per-user-isolated chats.
|
||||
assert seed_mock.call_args.kwargs.get("user_id") == "U_HUMAN"
|
||||
|
||||
|
||||
class TestMultiTargetDeliveryContinuesOnFailure:
|
||||
"""When delivery to one target fails inside the standalone thread-pool
|
||||
|
||||
Reference in New Issue
Block a user