From 3c52d3589fa5028c8a0faa90d809feed96af3cd0 Mon Sep 17 00:00:00 2001 From: Victor Kyriazakos Date: Wed, 19 Aug 2026 17:41:45 +0000 Subject: [PATCH] fix(cron): in_channel seed must not require the attach_to_session mirror opt-in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- cron/scheduler.py | 19 ++++++++++++++----- tests/cron/test_scheduler.py | 32 +++++++++++++++++++++++++++++--- 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/cron/scheduler.py b/cron/scheduler.py index 3540808691..bb19ed46da 100644 --- a/cron/scheduler.py +++ b/cron/scheduler.py @@ -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, diff --git a/tests/cron/test_scheduler.py b/tests/cron/test_scheduler.py index a1b8cd64e5..1224f10a0a 100644 --- a/tests/cron/test_scheduler.py +++ b/tests/cron/test_scheduler.py @@ -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