Skip to content
Closed
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
14 changes: 10 additions & 4 deletions cron/scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -1142,14 +1142,20 @@ def _resolve_single_delivery_target(job: dict, deliver_value: str) -> Optional[d
return None

if deliver_value == "origin":
if origin:
if origin and _is_known_delivery_platform(origin.get("platform", "")):
return {
"platform": origin["platform"],
"chat_id": str(origin["chat_id"]),
"thread_id": origin.get("thread_id"),
}
# Origin missing (e.g. job created via API/script) — try each
# platform's home channel as a fallback instead of silently dropping.
# Origin missing OR points at a platform that cannot receive cron
# delivery. The latter is the api_server trap (#69304): jobs created
# in an api_server session capture ``origin.platform="api_server"``,
# but that adapter's ``send()`` is a hardwired no-op (HTTP is
# request/response only). Trusting such an origin here makes the job
# look healthy (last_status=ok) while every fire silently fails to
# deliver. Treat an undeliverable origin like a missing one and fall
# back to configured home channels instead of silently dropping.
for platform_name in _iter_home_target_platforms():
chat_id = _get_home_target_chat_id(platform_name)
if chat_id:
Expand Down Expand Up @@ -1209,7 +1215,7 @@ def _resolve_single_delivery_target(job: dict, deliver_value: str) -> Optional[d
}

platform_name = deliver_value
if origin and origin.get("platform") == platform_name:
if origin and origin.get("platform") == platform_name and _is_known_delivery_platform(platform_name):
chat_id = _get_home_target_chat_id(platform_name)
if chat_id:
return {
Expand Down
74 changes: 74 additions & 0 deletions tests/cron/test_scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,23 @@
from tools.env_passthrough import clear_env_passthrough
from tools.credential_files import clear_credential_files

# Every env var the cron home-channel fallback can read. Clearing them all
# makes "no delivery target" assertions hermetic against a developer machine
# that happens to have one configured (e.g. WEIXIN_HOME_CHANNEL set globally).
_ALL_HOME_CHANNEL_ENV_VARS = (
"MATRIX_HOME_ROOM", "TELEGRAM_HOME_CHANNEL", "DISCORD_HOME_CHANNEL",
"SLACK_HOME_CHANNEL", "SIGNAL_HOME_CHANNEL", "MATTERMOST_HOME_CHANNEL",
"SMS_HOME_CHANNEL", "EMAIL_HOME_ADDRESS", "DINGTALK_HOME_CHANNEL",
"FEISHU_HOME_CHANNEL", "WECOM_HOME_CHANNEL", "WEIXIN_HOME_CHANNEL",
"BLUEBUBBLES_HOME_CHANNEL", "QQBOT_HOME_CHANNEL", "QQ_HOME_CHANNEL",
"WHATSAPP_HOME_CHANNEL", "WHATSAPP_CLOUD_HOME_CHANNEL",
)


def _clear_all_home_channels(monkeypatch):
for var in _ALL_HOME_CHANNEL_ENV_VARS:
monkeypatch.delenv(var, raising=False)


class TestPerJobToolsetMcpMerge:
"""A per-job enabled_toolsets allowlist must not silently drop MCP servers."""
Expand Down Expand Up @@ -492,6 +509,63 @@ def test_explicit_discord_channel_without_thread(self):
"thread_id": None,
}

def test_api_server_origin_does_not_pretend_to_deliver(self, monkeypatch):
"""#69304 — an api_server-origin job must NOT resolve deliver=origin to
the api_server itself. That adapter's send() is a hardwired no-op, so
trusting the origin would make the job look healthy (last_status=ok)
while every report silently failed to deliver. With no home channel
configured the target is unresolvable (None), not a fake api_server
target."""
_clear_all_home_channels(monkeypatch)

job = {
"deliver": "origin",
"origin": {"platform": "api_server", "chat_id": "sess-123"},
}
assert _resolve_delivery_target(job) is None

def test_api_server_origin_falls_back_to_home_channel(self, monkeypatch):
"""#69304 — an undeliverable api_server origin is treated like a missing
one: deliver=origin falls back to a configured home channel so the
report actually lands somewhere instead of being silently dropped."""
_clear_all_home_channels(monkeypatch)
monkeypatch.setenv("TELEGRAM_HOME_CHANNEL", "-4242")

job = {
"deliver": "origin",
"origin": {"platform": "api_server", "chat_id": "sess-123"},
}
assert _resolve_delivery_target(job) == {
"platform": "telegram",
"chat_id": "-4242",
"thread_id": None,
}

def test_bare_api_server_platform_with_matching_origin_resolves_to_nothing(self, monkeypatch):
"""#69304 — deliver='api_server' (origin matches) is not a usable
delivery target: the bare-platform branch must not trust an
undeliverable origin either."""
_clear_all_home_channels(monkeypatch)

job = {
"deliver": "api_server",
"origin": {"platform": "api_server", "chat_id": "sess-123"},
}
assert _resolve_delivery_target(job) is None

def test_deliverable_origin_still_resolves(self):
"""The undeliverable-origin guard must not break a normal deliverable
origin (telegram): deliver=origin still resolves to that chat."""
job = {
"deliver": "origin",
"origin": {"platform": "telegram", "chat_id": "-999", "thread_id": "42"},
}
assert _resolve_delivery_target(job) == {
"platform": "telegram",
"chat_id": "-999",
"thread_id": "42",
}

def test_list_form_deliver_is_normalized(self, monkeypatch):
"""deliver=['telegram'] (Python list) should resolve like 'telegram' string.

Expand Down
52 changes: 52 additions & 0 deletions tests/tools/test_cronjob_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,19 @@ def _setup_cron_dir(self, tmp_path, monkeypatch):
"HERMES_SESSION_CHAT_NAME",
):
monkeypatch.delenv(var, raising=False)
# Clear every gateway home channel so "no delivery target" assertions
# are hermetic against a developer machine that has one configured
# (e.g. WEIXIN_HOME_CHANNEL set globally would otherwise make the
# origin-fallback find a target and suppress the notice).
for var in (
"MATRIX_HOME_ROOM", "TELEGRAM_HOME_CHANNEL", "DISCORD_HOME_CHANNEL",
"SLACK_HOME_CHANNEL", "SIGNAL_HOME_CHANNEL", "MATTERMOST_HOME_CHANNEL",
"SMS_HOME_CHANNEL", "EMAIL_HOME_ADDRESS", "DINGTALK_HOME_CHANNEL",
"FEISHU_HOME_CHANNEL", "WECOM_HOME_CHANNEL", "WEIXIN_HOME_CHANNEL",
"BLUEBUBBLES_HOME_CHANNEL", "QQBOT_HOME_CHANNEL", "QQ_HOME_CHANNEL",
"WHATSAPP_HOME_CHANNEL", "WHATSAPP_CLOUD_HOME_CHANNEL",
):
monkeypatch.delenv(var, raising=False)
from gateway.session_context import clear_session_vars, set_session_vars

tokens = set_session_vars() # reset ContextVars to empty
Expand Down Expand Up @@ -657,6 +670,45 @@ def test_gateway_origin_no_notice(self, monkeypatch):
assert created["deliver"] == "origin"
assert "local-only cron job" not in created["message"]

def test_api_server_origin_emits_targeted_notice(self, monkeypatch):
"""#69304 — an api_server origin can't deliver; flag it at create time.

The api_server adapter's send() is a no-op, so deliver=origin would run
the job but silently drop every report. The create-time notice must
surface this with a platform-specific message (not the generic CLI/TUI
one) so the agent relays it instead of promising delivery.
"""
from gateway.session_context import set_session_vars

# No home channels configured (the autouse fixture clears them), so the
# origin-fallback also finds nothing.
set_session_vars(platform="api_server", chat_id="sess-123")
created = json.loads(
cronjob(action="create", prompt="x", schedule="every 2m")
)
assert created["success"] is True
assert created["deliver"] == "origin"
# Targeted message naming the undeliverable origin platform.
assert "api_server" in created["message"]
assert "cannot receive cron deliveries" in created["message"]
assert "deliver='telegram'" in created["message"]
# Not the generic CLI/TUI wording.
assert "local-only cron job" not in created["message"]

def test_api_server_origin_with_home_channel_no_notice(self, monkeypatch):
"""An api_server origin that falls back to a configured home channel
does deliver somewhere, so no notice is emitted."""
from gateway.session_context import set_session_vars

monkeypatch.setenv("TELEGRAM_HOME_CHANNEL", "-111")
set_session_vars(platform="api_server", chat_id="sess-123")
created = json.loads(
cronjob(action="create", prompt="x", schedule="every 2m")
)
assert created["success"] is True
assert "cannot receive cron deliveries" not in created["message"]
assert "local-only cron job" not in created["message"]


class TestValidateCronBaseUrl:
"""The cron base_url guard must not let a NAMED custom provider's stored
Expand Down
32 changes: 32 additions & 0 deletions tools/cronjob_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,23 @@ def _origin_from_env() -> Optional[Dict[str, str]]:
return None


def _origin_platform_can_deliver(platform: str) -> bool:
"""Whether a captured-origin platform can actually receive cron delivery.

Thin wrapper over ``cron.scheduler._is_known_delivery_platform`` so the
create-time notice stays in sync with the fire-time target resolver. The
api_server platform is the motivating case (#69304): it is a real gateway
platform with a working session, but its adapter ``send()`` is a no-op, so
an ``origin.platform="api_server"`` job can never deliver.
"""
try:
from cron.scheduler import _is_known_delivery_platform

return _is_known_delivery_platform(platform)
except Exception:
return True # don't second-guess on import failure; resolver will gate


def _local_delivery_notice(job: Dict[str, Any], user_deliver: Optional[str]) -> Optional[str]:
"""Return an informational notice when a created job won't deliver anywhere.

Expand Down Expand Up @@ -336,6 +353,21 @@ def _local_delivery_notice(job: Dict[str, Any], user_deliver: Optional[str]) ->
# If resolution can't be evaluated, fall back to the origin signal.
if job.get("origin"):
return None
origin = job.get("origin") or {}
if origin.get("platform") and not _origin_platform_can_deliver(origin["platform"]):
# The job was created in a session whose platform cannot receive cron
# delivery — the api_server trap (#69304): its adapter's send() is a
# no-op, so deliver=origin would run the job but silently drop every
# report. Point the caller at a real delivery channel.
return (
"This cron job's origin platform "
f"({origin['platform']}) cannot receive cron deliveries — its "
"output is saved (view it with cronjob(action='list')) but will "
"NOT be delivered back into that session. To be notified when it "
"runs, recreate or update the job with deliver set to a "
"gateway-connected platform, e.g. deliver='telegram' or "
"deliver='all'."
)
return (
"This is a local-only cron job: its output is saved (view it with "
"cronjob(action='list')) but will NOT be delivered back into this "
Expand Down