Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions cron/scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -1204,6 +1204,36 @@ def _deliver_result(job: dict, content: str, adapters=None, loop=None) -> Option
delivered = False
target_errors = []

# Continuable cron surface (D1/D2/D6): resolve the delivery surface for
# this platform generically from its config ``extra``. Default "thread"
# (today's behaviour, byte-identical). "in_channel" delivers the brief
# FLAT into the channel (no dedicated thread) so a plain channel reply
# continues the job in-context via the shared-channel session
# ``(platform, chat_id, None)`` — the same bucket ``reply_in_thread:
# false`` routes inbound channel messages to. The key is read
# generically here (any platform); the ``in_channel`` branch is gated on
# the adapter capability flag ``supports_inchannel_continuable`` so an
# unsupported platform fails SAFE to "thread" (Slack is the first
# consumer; "first consumer ≠ definition").
surface_mode = "thread"
try:
surface_raw = (pconfig.extra or {}).get("cron_continuable_surface")
if surface_raw is not None and str(surface_raw).strip().lower() == "in_channel":
surface_mode = "in_channel"
except Exception:
surface_mode = "thread"
in_channel_surface = surface_mode == "in_channel"
if in_channel_surface and runtime_adapter is not None and not getattr(
runtime_adapter, "supports_inchannel_continuable", False
):
# Fail safe (D6): platform has no in_channel continuation primitive.
logger.debug(
"Job '%s': cron_continuable_surface=in_channel not supported on "
"%s, using thread",
job.get("id", "?"), platform_name,
)
in_channel_surface = False

# Continuable cron (thread-preferred): when mirroring is enabled for the
# origin target and the gateway is live, try to open a DEDICATED thread
# for this job and deliver the brief into it. On thread-capable
Expand All @@ -1212,10 +1242,18 @@ def _deliver_result(job: dict, content: str, adapters=None, loop=None) -> Option
# continues with full context. On DM-only platforms (WhatsApp/Signal)
# create_handoff_thread returns None and we fall back to mirroring into
# the origin DM session (handled after delivery). Cf. _process_handoff.
#
# in_channel surface (D2): SKIP thread creation entirely — leave
# thread_id=None so the delivery posts flat, and let the existing
# origin-mirror (below) seed the shared-channel session (F5: for a
# channel-origin job with thread_id=None, _target_matches_origin matches
# and _maybe_mirror_cron_delivery seeds (platform, chat_id, None)). No
# new seed call is needed.
thread_seeded = False
opened_thread_id: Optional[str] = None
if (
mirror_this_target
and not in_channel_surface
and runtime_adapter is not None
and loop is not None
and not thread_id # never override an explicit origin thread/topic
Expand Down
2 changes: 2 additions & 0 deletions gateway/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -1008,6 +1008,8 @@ def _merge_platform_map(source_platforms: Any) -> None:
bridged["reply_prefix"] = platform_cfg["reply_prefix"]
if "reply_in_thread" in platform_cfg:
bridged["reply_in_thread"] = platform_cfg["reply_in_thread"]
if "cron_continuable_surface" in platform_cfg:
bridged["cron_continuable_surface"] = platform_cfg["cron_continuable_surface"]
if "require_mention" in platform_cfg:
bridged["require_mention"] = platform_cfg["require_mention"]
if plat == Platform.TELEGRAM and "allowed_chats" in platform_cfg:
Expand Down
15 changes: 15 additions & 0 deletions gateway/platforms/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -2257,6 +2257,21 @@ class BasePlatformAdapter(ABC):
# "typed_command_prefix", "/"); no per-platform branching at call sites.
typed_command_prefix: str = "/"

# Whether this adapter supports the ``in_channel`` continuable-cron surface
# (``platforms.<p>.extra.cron_continuable_surface: in_channel``): a
# continuable cron job delivered FLAT into a channel (no dedicated thread),
# with the user's plain channel reply continuing the job in-context via the
# shared-channel session. Only coherent on a platform that has BOTH a
# flat-reply outbound gate AND a whole-channel inbound session bucket keyed
# ``(platform, chat_id, None)`` — today that is Slack (``reply_in_thread:
# false``). Default False: an unsupported platform fails SAFE, treating
# ``in_channel`` as ``thread`` (a threaded continuation ≈ today's
# behaviour), never a dropped continuation. Read generically by the cron
# scheduler via ``getattr(adapter, "supports_inchannel_continuable",
# False)`` — no per-platform branching at the call site (the key stays a
# generic seam; Slack is merely the first consumer).
supports_inchannel_continuable: bool = False

def __init__(self, config: PlatformConfig, platform: Platform):
self.config = config
self.platform = platform
Expand Down
65 changes: 65 additions & 0 deletions plugins/platforms/slack/adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,14 @@ class SlackAdapter(BasePlatformAdapter):
# the prefix that works everywhere — instruction text must show it.
typed_command_prefix = "!"

# Slack has both halves the ``in_channel`` continuable-cron surface needs:
# a flat-reply outbound gate (``reply_in_thread: false`` → ``_resolve_thread_ts``
# returns None for top-level channel messages) AND a whole-channel inbound
# session bucket keyed ``(platform, channel_id, None)`` (the same
# ``reply_in_thread: false`` path in ``_handle_slack_message``). So a
# continuable cron delivered flat here continues in-context on a plain reply.
supports_inchannel_continuable = True

def __init__(self, config: PlatformConfig):
super().__init__(config, Platform.SLACK)
self._app: Optional[Any] = None
Expand Down Expand Up @@ -1068,6 +1076,7 @@ async def connect(self, *, is_reconnect: bool = False) -> bool:

self._warn_if_missing_group_dm_scopes(auth_response, team_name)
self._warn_if_not_bot_token(auth_response, team_name)
self._warn_if_inchannel_without_flat_reply(team_name)

# Register message event handler
@self._app.event("message")
Expand Down Expand Up @@ -1539,6 +1548,62 @@ def _dm_top_level_threads_as_sessions(self) -> bool:
return True # default: each DM thread is its own session
return str(raw).strip().lower() in {"1", "true", "yes", "on"}

def _cron_continuable_surface(self) -> str:
"""Resolve the continuable-cron delivery surface for this platform.

Values: ``"thread"`` (default — today's behaviour: a continuable cron
job opens a dedicated hidden thread and seeds it) or ``"in_channel"``
(deliver FLAT into the channel timeline; the shared-channel session
``(slack, channel_id, None)`` is the continuation surface). Set
``platforms.slack.extra.cron_continuable_surface: in_channel`` in
config.yaml. Pair with ``reply_in_thread: false`` so the user's reply
is answered flat in the channel and keyed to the same shared session —
see ``_warn_if_inchannel_without_flat_reply``. Any unrecognised value
coerces to ``"thread"`` (fail safe).
"""
raw = self.config.extra.get("cron_continuable_surface")
if raw is None:
return "thread"
val = str(raw).strip().lower()
return "in_channel" if val == "in_channel" else "thread"

def _warn_if_inchannel_without_flat_reply(self, team_name: str) -> None:
"""Warn when ``in_channel`` is set without the required ``reply_in_thread: false`` pairing.

The two knobs are orthogonal (D4/D5): ``cron_continuable_surface:
in_channel`` skips thread creation on delivery, and ``reply_in_thread:
false`` makes the bot answer inbound channel messages flat and key them
to the whole-channel session ``(slack, channel_id, None)``. For a
continuable in-channel cron to actually continue on a plain reply, BOTH
must hold: the seed lands in the shared-channel session, and the reply
must resolve to (and be answered in) that same flat session.

Enforcement is WARN, not hard-require (D5): the misconfiguration fails
SAFE — ``in_channel`` without ``reply_in_thread: false`` yields a
threaded continuation (≈ today's behaviour), never a dropped/orphaned
session — so a config-load rejection would be heavier than warranted
and would make the two knobs non-orthogonal. Mirrors the existing
connect-time warning pattern (``_warn_if_missing_group_dm_scopes``,
``_warn_if_not_bot_token``).
"""
try:
if self._cron_continuable_surface() != "in_channel":
return
# reply_in_thread defaults True (legacy: reply in a thread).
if self.config.extra.get("reply_in_thread", True):
logger.warning(
"[Slack] %s: cron_continuable_surface=in_channel is set "
"WITHOUT reply_in_thread=false. A continuable in-channel "
"cron job will deliver flat, but the bot will still reply "
"to your continuation in a thread — so it falls back to a "
"threaded continuation (\u2248 default behaviour), not the "
"flat channel session you asked for. Set "
"platforms.slack.extra.reply_in_thread: false to pair them.",
team_name,
)
except Exception:
pass

def _resolve_thread_ts(
self,
reply_to: Optional[str] = None,
Expand Down
131 changes: 131 additions & 0 deletions tests/cron/test_scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -3997,3 +3997,134 @@ def test_seed_thread_session_noop_on_empty_text(self):
)
store.get_or_create_session.assert_not_called()
mirror_mock.assert_not_called()


class TestCronContinuableSurfaceInChannel:
"""cron_continuable_surface: in_channel — deliver a continuable cron FLAT
into a channel (no dedicated thread), so a plain channel reply continues the
job via the shared-channel session (platform, chat_id, None).

Design: decisions.md D1/D2/D6 + F5. The scheduler reads the per-platform key
generically from pconfig.extra; the in_channel branch is gated on the
adapter capability flag ``supports_inchannel_continuable`` (Slack=True,
others fail SAFE to thread). In in_channel mode the thread-open branch is
SKIPPED (thread_id stays None), so the existing origin-mirror seeds the
shared-channel session — no new seed code (G6).
"""

def _slack_cfg(self, extra):
"""A mock GatewayConfig with a Slack pconfig carrying ``extra``."""
from gateway.config import Platform

pconfig = MagicMock()
pconfig.enabled = True
pconfig.extra = extra
mock_cfg = MagicMock()
mock_cfg.platforms = {Platform.SLACK: pconfig}
return mock_cfg

def _run_inchannel_delivery(self, extra, adapter, *, mirror_ok=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."""
from gateway.config import Platform
from concurrent.futures import Future

mock_cfg = self._slack_cfg(extra)

loop = MagicMock()
loop.is_running.return_value = True

def fake_run_coro(coro, _loop):
future = Future()
try:
import asyncio as _asyncio
future.set_result(_asyncio.run(coro))
except BaseException as _e: # noqa: BLE001
future.set_exception(_e)
return future

job = {
"id": "brief-job",
"name": "Daily Brief",
"deliver": "origin",
# Channel origin: no thread_id (flat channel message scheduled it).
"origin": {"platform": "slack", "chat_id": "C123"},
# Opt into the continuable mirror.
"attach_to_session": True,
}

with patch("gateway.config.load_gateway_config", return_value=mock_cfg), \
patch("cron.scheduler.load_config", return_value={"cron": {"wrap_response": False}}), \
patch("cron.scheduler._open_continuable_cron_thread") as open_thread_mock, \
patch("asyncio.run_coroutine_threadsafe", side_effect=fake_run_coro), \
patch("gateway.mirror.mirror_to_session", return_value=mirror_ok) as mirror_mock:
_deliver_result(
job, "Here is today's brief.",
adapters={Platform.SLACK: adapter}, loop=loop,
)
return open_thread_mock, mirror_mock

def _slack_adapter(self, supports_inchannel=True):
adapter = AsyncMock()
adapter.send.return_value = MagicMock(success=True)
# Capability flag read via getattr in the scheduler.
adapter.supports_inchannel_continuable = supports_inchannel
return adapter

def test_in_channel_skips_thread_open(self):
"""G2: in_channel mode must NOT open a handoff thread."""
adapter = self._slack_adapter(supports_inchannel=True)
open_thread_mock, _ = self._run_inchannel_delivery(
{"cron_continuable_surface": "in_channel"}, adapter,
)
open_thread_mock.assert_not_called()

def test_in_channel_seeds_shared_channel_session_flat(self):
"""G3/F5: with the thread-open branch skipped, the existing origin-mirror
seeds the shared-channel session with thread_id=None (flat)."""
adapter = self._slack_adapter(supports_inchannel=True)
_, mirror_mock = self._run_inchannel_delivery(
{"cron_continuable_surface": "in_channel"}, adapter,
)
mirror_mock.assert_called_once()
# Seeded flat: no thread_id → session (slack, C123, None).
assert mirror_mock.call_args.kwargs.get("thread_id") is None
assert mirror_mock.call_args[0][0] == "slack"
assert mirror_mock.call_args[0][1] == "C123"
assert "Here is today's brief." in mirror_mock.call_args[0][2]

def test_thread_mode_default_still_opens_thread(self):
"""G1 regression: the default (thread) mode is byte-identical — the
thread-open branch still fires when no surface key is set."""
adapter = self._slack_adapter(supports_inchannel=True)
open_thread_mock, _ = self._run_inchannel_delivery({}, adapter)
open_thread_mock.assert_called_once()

def test_explicit_thread_value_opens_thread(self):
"""An explicit cron_continuable_surface: thread is the default path."""
adapter = self._slack_adapter(supports_inchannel=True)
open_thread_mock, _ = self._run_inchannel_delivery(
{"cron_continuable_surface": "thread"}, adapter,
)
open_thread_mock.assert_called_once()

def test_in_channel_on_unsupported_platform_fails_safe_to_thread(self):
"""D6 fail-safe: in_channel on an adapter WITHOUT the capability flag
falls back to the thread path (a threaded continuation ≈ today), never
a dropped continuation."""
adapter = self._slack_adapter(supports_inchannel=False)
open_thread_mock, _ = self._run_inchannel_delivery(
{"cron_continuable_surface": "in_channel"}, adapter,
)
# Capability absent → treated as thread → thread-open still attempted.
open_thread_mock.assert_called_once()

def test_unrecognised_surface_value_coerces_to_thread(self):
"""Any non-'in_channel' value is the default thread path (fail safe)."""
adapter = self._slack_adapter(supports_inchannel=True)
open_thread_mock, _ = self._run_inchannel_delivery(
{"cron_continuable_surface": "bogus"}, adapter,
)
open_thread_mock.assert_called_once()

Loading
Loading