From 751a300fca914c6ebabf046b512d4abe7658046b Mon Sep 17 00:00:00 2001 From: Ben Date: Wed, 1 Jul 2026 19:01:57 +1000 Subject: [PATCH] docs(cron): scope in_channel to channels; document DM continuation knob MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live DM testing showed a reply to a DM cron brief did NOT continue the job. Root cause: for a 1:1 DM the governing knob is dm_top_level_threads_as_sessions (default True), NOT reply_in_thread / cron_continuable_surface. Under the default, each top-level DM keys to a per-message session (…:dm::), so a reply mints a new ts and can never converge with the flat …:dm: session the cron seed creates. A 1:1 DM has no thread-vs-timeline split, so "in_channel" has no coherent meaning for a DM — cron_continuable_surface is a channel concept and is a no-op for DMs. DM continuation is governed entirely by dm_top_level_threads_as_sessions: - false → all top-level DMs share …:dm: → seed + reply converge → works - true (default) → per-message sessions → no continuation (cron or interactive) Option A (chosen): document the requirement; no code change (the flat-DM seed from the prior commit already lands correctly when the knob is false). Adds a ":::note 1:1 DMs" admonition to cron.md + the zh-Hans mirror. Verification (real inbound handler, not a hard-coded assumption — the mistake that made the earlier DM E2E falsely pass): tests/manual/cron_inchannel_dm_e2e.py drives the REAL _handle_slack_message for a top-level DM under both knob values and asserts false→converges (…:dm:D_TESTDM == seed), true→diverges (…:dm:D_TESTDM:). See decisions.md D9. --- tests/manual/cron_inchannel_dm_e2e.py | 119 ++++++++++++++++++ website/docs/user-guide/features/cron.md | 18 +++ .../current/user-guide/features/cron.md | 15 +++ 3 files changed, 152 insertions(+) create mode 100644 tests/manual/cron_inchannel_dm_e2e.py diff --git a/tests/manual/cron_inchannel_dm_e2e.py b/tests/manual/cron_inchannel_dm_e2e.py new file mode 100644 index 000000000..16273ce1a --- /dev/null +++ b/tests/manual/cron_inchannel_dm_e2e.py @@ -0,0 +1,119 @@ +""" +DM-path verification for in_channel continuable cron (Option A scoping). + +Option A: `cron_continuable_surface` is a CHANNEL feature. For a 1:1 DM the +governing knob is the pre-existing `dm_top_level_threads_as_sessions` — a DM has +no thread-vs-timeline split, so DM continuation works ONLY when top-level DMs +share one flat session (`dm_top_level_threads_as_sessions: false`). + +This harness PROVES that scoping against the REAL inbound handler +(`SlackAdapter._handle_slack_message`) — no hard-coded thread_id assumption (the +mistake that made the earlier E2E falsely pass): + + SCENARIO 1 (the supported config, false): a top-level DM reply keys to the + flat `…:dm:` session — the SAME key the cron seed + (`_seed_cron_channel_session`, is_dm=True) creates. → continuation works. + + SCENARIO 2 (the default, True — CONTROL): a top-level DM reply keys to a + per-message `…:dm::` session — DIVERGES from the flat seed. → this + is exactly why in_channel does NOT give DM continuation under the default, + and why Option A documents the requirement rather than pretending otherwise. + +Run from INSIDE the worktree: + cd + PYTHONPATH="$PWD" ../../.venv/bin/python tests/manual/cron_inchannel_dm_e2e.py + +No real names. Uses a throwaway HERMES_HOME. +""" + +import asyncio +import os +import sys +import tempfile +from pathlib import Path +from unittest.mock import AsyncMock, MagicMock, patch + +os.environ["HERMES_HOME"] = tempfile.mkdtemp(prefix="cron_dm_e2e_") + +import cron.scheduler as sched # noqa: E402 +from gateway.config import PlatformConfig, Platform # noqa: E402 +from gateway.session import build_session_key, SessionSource # noqa: E402 +from plugins.platforms.slack.adapter import SlackAdapter # noqa: E402 + +DM_CHAT = "D_TESTDM" +BOT = "U_TESTBOT" +USER = "U_TESTER" + + +async def _inbound_dm_reply_key(dm_threads_as_sessions: bool): + """Drive the REAL _handle_slack_message for a top-level DM message and + return (session_key, source) the dispatched MessageEvent resolves to.""" + cfg = PlatformConfig(enabled=True, token="xoxb-test-not-real") + cfg.extra["dm_top_level_threads_as_sessions"] = dm_threads_as_sessions + a = SlackAdapter(cfg) + a._app = MagicMock() + a._app.client = AsyncMock() + a._bot_user_id = BOT + a._running = True + + captured = [] + a.handle_message = AsyncMock(side_effect=lambda e: captured.append(e)) + + event = { + "channel": DM_CHAT, + "channel_type": "im", # 1:1 DM + "user": USER, + "text": "how many items in that brief?", + "ts": "1782999999.000100", # a NEW top-level DM message (no thread_ts) + } + with patch.object(a, "_resolve_user_name", new=AsyncMock(return_value="tester")): + await a._handle_slack_message(event) + + assert len(captured) == 1, "DM reply was dropped by the handler" + src = captured[0].source + return build_session_key(src), src + + +def _seed_key() -> str: + """The session key the cron in_channel DM seed creates (is_dm=True, flat).""" + seed_source = SessionSource( + platform=Platform.SLACK, chat_id=DM_CHAT, chat_type="dm", + user_id=USER, thread_id=None, + ) + return build_session_key(seed_source) + + +def main(): + print(f"adapter module: {SlackAdapter.__module__} ({sched.__file__.rsplit('/',2)[0]})") + seed_key = _seed_key() + print(f"\ncron in_channel DM seed key: {seed_key}") + + # SCENARIO 1 — supported config (false): reply MUST converge on the seed. + key_false, src_false = asyncio.run(_inbound_dm_reply_key(False)) + print(f"\n[dm_top_level_threads_as_sessions=false] reply key: {key_false}") + print(f" thread_id on reply source: {src_false.thread_id!r}") + assert key_false == seed_key, ( + f"FAIL: with the supported config, reply key {key_false} != seed {seed_key}" + ) + print(" ✓ CONVERGES with the seed → DM continuation works") + + # SCENARIO 2 — default (true): reply DIVERGES (this is why A documents the req). + key_true, src_true = asyncio.run(_inbound_dm_reply_key(True)) + print(f"\n[dm_top_level_threads_as_sessions=true (default)] reply key: {key_true}") + print(f" thread_id on reply source: {src_true.thread_id!r}") + assert key_true != seed_key, ( + "unexpected: default DM keying matched the flat seed — the control is wrong" + ) + print(" ✓ DIVERGES from the seed (per-message session) → in_channel gives") + print(" NO DM continuation under the default; false is required (Option A)") + + print( + "\nPASS: Option A verified against the REAL inbound handler.\n" + " • DM continuable cron works IFF dm_top_level_threads_as_sessions: false\n" + " (reply and seed converge on the flat …:dm: session).\n" + " • Under the default (true) they diverge — documented, not silently broken." + ) + + +if __name__ == "__main__": + main() diff --git a/website/docs/user-guide/features/cron.md b/website/docs/user-guide/features/cron.md index b1f903d8d..a65a4bca1 100644 --- a/website/docs/user-guide/features/cron.md +++ b/website/docs/user-guide/features/cron.md @@ -370,6 +370,24 @@ to the `thread` surface (their continuation primitives differ); the choice is per-platform, set under each platform's config. It's a gateway-side config flag — a `/restart` picks it up; no Slack app reinstall is needed. +:::note 1:1 DMs +`cron_continuable_surface` is a **channel** setting — a 1:1 DM has no +thread-vs-timeline split to choose between (the DM is already flat), so the key +has no effect there. What governs whether a DM cron delivery is continuable is +the separate, pre-existing knob **`slack.dm_top_level_threads_as_sessions`**: + +- **`false`** — all top-level DMs share one rolling DM session, so a continuable + cron brief and your reply land in the **same** session and the job continues in + context. This is what you want for continuable cron in a DM. +- **`true`** (default) — each top-level DM message is its own session, so a reply + to a delivered brief starts a *fresh* session that has no record of the brief. + Continuation does not work in this mode (for cron or any other flat delivery). + +So for a continuable cron job delivered to a 1:1 DM, set +`slack.dm_top_level_threads_as_sessions: false`. `cron_continuable_surface` is +not required (and is ignored) for DMs. +::: + ### Silent suppression If the agent's final response contains `[SILENT]`, delivery is suppressed entirely. The output is still saved locally for audit (in `~/.hermes/cron/output/`), but no message is sent to the delivery target. diff --git a/website/i18n/zh-Hans/docusaurus-plugin-content-docs/current/user-guide/features/cron.md b/website/i18n/zh-Hans/docusaurus-plugin-content-docs/current/user-guide/features/cron.md index 1df1a55aa..e543f8cfc 100644 --- a/website/i18n/zh-Hans/docusaurus-plugin-content-docs/current/user-guide/features/cron.md +++ b/website/i18n/zh-Hans/docusaurus-plugin-content-docs/current/user-guide/features/cron.md @@ -376,6 +376,21 @@ in_channel 任务——都会加入同一段滚动对话。这是「平铺在频 不同）；该选择按平台设置，位于各平台的配置下。这是网关侧的配置项——`/restart` 即可 生效；无需重新安装 Slack 应用。 +:::note 1:1 私信（DM） +`cron_continuable_surface` 是**频道**设置——1:1 私信没有「话题 vs 时间线」的区分 +（私信本身就是平铺的），因此该键在私信中无效。决定私信 cron 投递是否可继续的是另一个 +已有的独立开关 **`slack.dm_top_level_threads_as_sessions`**： + +- **`false`**——所有顶层私信共享同一个滚动私信会话，因此可继续的 cron 简报与你的回复 + 落在**同一个**会话里，任务得以带上下文继续。这正是私信中可继续 cron 所需要的。 +- **`true`**（默认）——每条顶层私信消息各自成为独立会话，因此对已投递简报的回复会开启 + 一个**全新**、不含该简报记录的会话。此模式下继续功能不可用（对 cron 或任何平铺投递皆然）。 + +所以，若要让 cron 任务在 1:1 私信中可继续，请设置 +`slack.dm_top_level_threads_as_sessions: false`。私信不需要（也会忽略） +`cron_continuable_surface`。 +::: + ### 静默抑制 如果 agent 的最终响应以 `[SILENT]` 开头，投递将被完全抑制。输出仍会保存到本地以供审计（位于 `~/.hermes/cron/output/`），但不会向投递目标发送任何消息。