fix(cron): make user-written bare-platform home targets continuable
A managed cron with `deliver: "slack"` (no captured origin) delivers to the Slack home channel, but the brief was never mirrored into that session and the in_channel seed never fired: bare-platform targets resolved with no `_resolved_from` provenance, so `_target_mirror_eligible` treated them like `all` broadcast expansions. - `_resolve_delivery_targets` derives `from_broadcast` from the raw token and passes it to `_resolve_single_delivery_target`; a user-written bare platform token is tagged `_resolved_from: "home"`, `all` expansions stay untagged (fan-out is never continuable). - `_target_mirror_eligible`: `home` eligible under the same flags as `origin_fallback` (per-job attach_to_session wins, else cron.mirror_delivery). - `_MIRROR_PROVENANCE_RANK`: `home` == `origin_fallback` so token order through dedup cannot strip eligibility. - Docs, tool schema text, cron/gateway AGENTS.md updated; existing exact-dict pins carry the new key. Squashed from PR #101819 (commits 5fe612c9, 65e36926, 10e0af1d, f2cc4639): they predate the cron/scheduler.py -> scheduler_delivery.py split and do not cherry-pick individually onto current main.
This commit is contained in:
committed by
kshitij
parent
16a408534c
commit
22356075f1
+4
-2
@@ -19,8 +19,10 @@ Hardening invariants — each guards a real failure; don't weaken without answer
|
||||
- Catch-up window = half the period, clamped to 120s–2h; 120s grace for missed one-shots.
|
||||
- File lock `~/.hermes/cron/.tick.lock` prevents duplicate ticks across processes.
|
||||
- Cron sessions pass `skip_memory=True`; memory providers intentionally do not run during cron.
|
||||
- Deliveries are **not mirrored** into the target gateway session — they land in their own cron
|
||||
session with a header/footer frame so the main conversation's role alternation stays intact.
|
||||
- Cron execution has its own session. Eligible continuable deliveries may mirror or seed the
|
||||
reply-facing conversation: origin, origin-less home fallback, user-written bare-platform home,
|
||||
or opted-in explicit targets. `all` expansions do not gain home mirror eligibility. Mirrored
|
||||
briefs are labelled user turns appended at a turn boundary, preserving role alternation.
|
||||
- The cron ticker runs in the desktop-spawned backend when `HERMES_DESKTOP=1` — that env var means
|
||||
"spawned by the app", not "a GUI is watching" (root: capability is a property of the session).
|
||||
- Background `delegate_task` is process-local; work that must survive restarts is a cron job or a
|
||||
|
||||
+42
-31
@@ -127,17 +127,19 @@ def _target_matches_origin(origin: dict, platform_name: str, chat_id: str,
|
||||
|
||||
# Provenance rank for the dedup OR-merge in _resolve_delivery_targets (higher = stronger mirror
|
||||
# claim). Broadcasts rank 0 so "origin,all"/"all,origin" keep the origin tag regardless of order.
|
||||
_MIRROR_PROVENANCE_RANK = {"origin": 3, "origin_fallback": 2, "explicit": 1}
|
||||
_MIRROR_PROVENANCE_RANK = {"origin": 3, "origin_fallback": 2, "home": 2, "explicit": 1}
|
||||
|
||||
|
||||
def _target_mirror_eligible(
|
||||
job: dict, target: dict, *, global_mirror: bool, origin_match: Optional[bool] = None) -> bool:
|
||||
"""Whether a resolved delivery target may receive the transcript mirror. Origin targets:
|
||||
always. ``origin_fallback`` (deliver=origin with no captured origin → home channel, standing
|
||||
in for the primary conversation): same flags as a true origin. ``explicit``
|
||||
``platform:chat_id``: ONLY with per-job ``attach_to_session: true`` — the global flag must
|
||||
never write transcripts into arbitrary explicitly-addressed chats. Untagged broadcasts
|
||||
(``all``, bare-platform home) are never eligible. ``origin_match`` may be precomputed."""
|
||||
in for the primary conversation) and ``home`` (user-written bare-platform token, e.g.
|
||||
``deliver: slack`` — deliberately addresses that platform's home channel): same flags as a
|
||||
true origin. ``explicit`` ``platform:chat_id``: ONLY with per-job ``attach_to_session: true``
|
||||
— the global flag must never write transcripts into arbitrary explicitly-addressed chats.
|
||||
Untagged broadcast expansions (``all``) are never eligible. ``origin_match`` may be
|
||||
precomputed."""
|
||||
if origin_match is None:
|
||||
origin = _resolve_origin(job) or {}
|
||||
origin_match = _target_matches_origin(
|
||||
@@ -145,7 +147,7 @@ def _target_mirror_eligible(
|
||||
if origin_match:
|
||||
return True
|
||||
resolved_from = target.get("_resolved_from")
|
||||
if resolved_from == "origin_fallback":
|
||||
if resolved_from in ("origin_fallback", "home"):
|
||||
# Same precedence as _cron_mirror_delivery_enabled (keep in sync): a per-job False must
|
||||
# beat a global True even for callers that don't pre-merge `global_mirror`.
|
||||
per_job = job.get("attach_to_session")
|
||||
@@ -557,8 +559,15 @@ def _home_target(platform_name: str, chat_id: str, resolved_from: Optional[str]
|
||||
return target
|
||||
|
||||
|
||||
def _resolve_single_delivery_target(job: dict, deliver_value: str) -> Optional[dict]:
|
||||
"""Resolve one concrete auto-delivery target for a cron job."""
|
||||
def _resolve_single_delivery_target(
|
||||
job: dict, deliver_value: str, *, from_broadcast: bool = False
|
||||
) -> Optional[dict]:
|
||||
"""Resolve one concrete auto-delivery target for a cron job.
|
||||
|
||||
``from_broadcast`` marks a bare-platform token that was produced by expanding a broadcast
|
||||
token (``all``) rather than written by the user; broadcast expansions carry no mirror
|
||||
provenance (fan-out is never continuable), while a user-written bare platform token is a
|
||||
deliberate home-channel address and gets the ``home`` tag."""
|
||||
origin = _resolve_origin(job)
|
||||
if deliver_value == "local":
|
||||
return None
|
||||
@@ -615,10 +624,13 @@ def _resolve_single_delivery_target(job: dict, deliver_value: str) -> Optional[d
|
||||
"_resolved_from": "explicit", # mirror-eligible only under attach_to_session opt-in
|
||||
}
|
||||
platform_name = deliver_value
|
||||
home_provenance = None if from_broadcast else "home"
|
||||
if origin and origin.get("platform") == platform_name:
|
||||
chat_id = _get_home_target_chat_id(platform_name)
|
||||
if chat_id:
|
||||
return _home_target(platform_name, chat_id)
|
||||
return _home_target(platform_name, chat_id, home_provenance)
|
||||
# No home configured: falls back to the origin chat. No tag needed — the
|
||||
# origin-match check in _target_mirror_eligible already covers this target.
|
||||
return {
|
||||
"platform": platform_name,
|
||||
"chat_id": str(origin["chat_id"]),
|
||||
@@ -627,7 +639,7 @@ def _resolve_single_delivery_target(job: dict, deliver_value: str) -> Optional[d
|
||||
if not _is_known_delivery_platform(platform_name):
|
||||
return None
|
||||
chat_id = _get_home_target_chat_id(platform_name)
|
||||
return _home_target(platform_name, chat_id) if chat_id else None
|
||||
return _home_target(platform_name, chat_id, home_provenance) if chat_id else None
|
||||
|
||||
|
||||
def _get_bot_chat_delivery_timeout() -> int:
|
||||
@@ -853,29 +865,28 @@ def _resolve_delivery_targets(job: dict, *, for_failure: bool = False) -> List[d
|
||||
if deliver == "local":
|
||||
return []
|
||||
|
||||
parts: List[str] = []
|
||||
for raw in deliver.split(","):
|
||||
if raw.strip():
|
||||
parts.extend(_expand_routing_tokens(raw.strip()))
|
||||
|
||||
seen = {}
|
||||
targets = []
|
||||
for part in parts:
|
||||
target = _resolve_single_delivery_target(job, part)
|
||||
if not target:
|
||||
for raw in deliver.split(","):
|
||||
raw = raw.strip()
|
||||
if not raw:
|
||||
continue
|
||||
key = (target["platform"].lower(), str(target["chat_id"]), target.get("thread_id"))
|
||||
kept = seen.get(key)
|
||||
if kept is None:
|
||||
seen[key] = target
|
||||
targets.append(target)
|
||||
elif (
|
||||
# OR-merge provenance on dedup: "origin,all" in either order must keep the
|
||||
# origin/origin_fallback tag or mirror eligibility would depend on token order.
|
||||
_MIRROR_PROVENANCE_RANK.get(str(target.get("_resolved_from") or ""), 0)
|
||||
> _MIRROR_PROVENANCE_RANK.get(str(kept.get("_resolved_from") or ""), 0)
|
||||
):
|
||||
kept["_resolved_from"] = target.get("_resolved_from")
|
||||
from_broadcast = raw.lower() in _ROUTING_TOKENS
|
||||
for part in _expand_routing_tokens(raw):
|
||||
target = _resolve_single_delivery_target(job, part, from_broadcast=from_broadcast)
|
||||
if not target:
|
||||
continue
|
||||
key = (target["platform"].lower(), str(target["chat_id"]), target.get("thread_id"))
|
||||
kept = seen.get(key)
|
||||
if kept is None:
|
||||
seen[key] = target
|
||||
targets.append(target)
|
||||
elif (
|
||||
# Keep origin/origin_fallback/home provenance regardless of broadcast token order.
|
||||
_MIRROR_PROVENANCE_RANK.get(str(target.get("_resolved_from") or ""), 0)
|
||||
> _MIRROR_PROVENANCE_RANK.get(str(kept.get("_resolved_from") or ""), 0)
|
||||
):
|
||||
kept["_resolved_from"] = target.get("_resolved_from")
|
||||
return targets
|
||||
|
||||
|
||||
@@ -1542,7 +1553,7 @@ def _prepare_target_delivery(
|
||||
"Job '%s': delivering to %s:%s thread_id=%s",
|
||||
job["id"], platform_name, chat_id, thread_id)
|
||||
|
||||
# Mirror: origin, home FALLBACK for origin-less deliver=origin, or attach_to_session opt-in.
|
||||
# Mirror: origin, origin-less home fallback, user-written home, or explicit-target opt-in.
|
||||
origin_target = _target_matches_origin(origin, platform_name, chat_id, thread_id)
|
||||
mirror_this_target = mirror_enabled and _target_mirror_eligible(
|
||||
job, target, global_mirror=mirror_enabled, origin_match=origin_target)
|
||||
|
||||
+4
-2
@@ -75,8 +75,10 @@ acceptance. Refused admission refunds every claimed batch sibling without spendi
|
||||
actual delivery errors keep their bounded retry policy. Recognized raw API routes resolve after
|
||||
persisted messaging origins and defer quietly when unavailable; malformed routes still warn.
|
||||
|
||||
Cron deliveries are NOT mirrored into the target gateway session — they land in their own cron
|
||||
session with a header/footer frame so the main conversation's role alternation stays intact
|
||||
Cron execution has its own session. Eligible continuable deliveries may mirror or seed the
|
||||
reply-facing conversation: origin, origin-less home fallback, user-written bare-platform home,
|
||||
or opted-in explicit targets. `all` expansions do not gain home mirror eligibility. Mirrored
|
||||
briefs are labelled user turns appended at a turn boundary, preserving role alternation
|
||||
(`cron/AGENTS.md`).
|
||||
|
||||
## Gateway lifecycle vs. the Desktop app
|
||||
|
||||
@@ -1667,9 +1667,10 @@ DEFAULT_CONFIG = {
|
||||
# Make cron deliveries CONTINUABLE (user can reply to a brief with it in context). False
|
||||
# keeps deliveries isolated to the job's session; per-job `attach_to_session` overrides.
|
||||
# Thread-capable platforms (Telegram topics, Discord/Slack threads) get a seeded thread per
|
||||
# job via create_handoff_thread; DM-only platforms mirror the brief into the origin DM
|
||||
# job via create_handoff_thread; DM-only platforms mirror the brief into the target DM
|
||||
# session. Appended at a turn boundary via mirror_to_session, cached system prompt
|
||||
# untouched; fan-out/broadcast targets are never mirrored.
|
||||
# untouched. User-written bare platforms address home conversations, unlike `all`
|
||||
# broadcast expansions, which do not gain mirror eligibility.
|
||||
"mirror_delivery": False,
|
||||
# Max due jobs run in parallel per tick. None/0 = unbounded (thread count only); 1 = serial.
|
||||
# Env override: HERMES_CRON_MAX_PARALLEL.
|
||||
|
||||
@@ -13,12 +13,13 @@ origin-FALLBACK target is not a broadcast: it is the best available stand-in
|
||||
for the user's primary conversation.
|
||||
|
||||
Design under test:
|
||||
- Delivery targets carry a `mirror_eligibility` tag set at resolution time:
|
||||
- Delivery targets carry `_resolved_from` provenance used to determine mirror eligibility:
|
||||
* origin match -> eligible (unchanged)
|
||||
* origin-fallback (deliver=origin, no origin) -> eligible (NEW)
|
||||
* explicit platform:chat -> eligible ONLY with per-job attach_to_session
|
||||
(NEW, opt-in; the global flag never activates explicit targets)
|
||||
* `all` / bare-platform expansion -> never eligible (unchanged invariant)
|
||||
* user-written bare platform -> home conversation, same flags as origin-fallback
|
||||
* `all` broadcast expansion -> never eligible (unchanged invariant)
|
||||
- Dedup across tokens (e.g. "origin,all" hitting the same chat) OR-merges
|
||||
eligibility so token order cannot strip it.
|
||||
- The in_channel flat-session seed requires a DM-shaped target or a known
|
||||
@@ -63,14 +64,44 @@ class TestMirrorEligibilityResolution:
|
||||
job = {"deliver": "all", "origin": None}
|
||||
targets = _resolve_delivery_targets(job)
|
||||
assert targets, "home channel should expand from 'all'"
|
||||
from cron.scheduler_delivery import _expand_routing_tokens
|
||||
|
||||
# Expansion is string-only routing, independent of mirror eligibility.
|
||||
assert "slack" in _expand_routing_tokens("ALL")
|
||||
assert _expand_routing_tokens("slack") == ["slack"]
|
||||
for t in targets:
|
||||
assert not _target_mirror_eligible(job, t, global_mirror=True)
|
||||
assert not _target_mirror_eligible(
|
||||
{**job, "attach_to_session": True}, t, global_mirror=True
|
||||
)
|
||||
|
||||
def test_bare_platform_target_is_not_eligible(self):
|
||||
def test_bare_platform_target_is_eligible_as_home(self):
|
||||
"""A user-WRITTEN bare-platform token deliberately addresses the
|
||||
home channel — same conversation semantics as origin_fallback,
|
||||
so the same eligibility flags apply (field report 2026-09-02:
|
||||
managed ``deliver: slack`` crons delivered but never injected).
|
||||
Broadcast expansions from ``all`` remain ineligible — see
|
||||
test_all_expansion_is_never_eligible."""
|
||||
job = {"deliver": "slack", "origin": None}
|
||||
targets = _resolve_delivery_targets(job)
|
||||
assert len(targets) == 1
|
||||
assert not _target_mirror_eligible(job, targets[0], global_mirror=True)
|
||||
assert _target_mirror_eligible(job, targets[0], global_mirror=True)
|
||||
assert not _target_mirror_eligible(job, targets[0], global_mirror=False)
|
||||
assert _target_mirror_eligible(
|
||||
{**job, "attach_to_session": True}, targets[0], global_mirror=False
|
||||
)
|
||||
assert not _target_mirror_eligible(
|
||||
{**job, "attach_to_session": False}, targets[0], global_mirror=True
|
||||
)
|
||||
|
||||
@pytest.mark.parametrize("deliver", ["slack,all", "all,slack", " ALL , slack "])
|
||||
def test_dedup_bare_and_all_keeps_home_eligibility(self, deliver):
|
||||
"""An explicit home address stays continuable regardless of broadcast token order."""
|
||||
job = {"deliver": deliver, "origin": None}
|
||||
targets = _resolve_delivery_targets(job)
|
||||
slack_targets = [t for t in targets if t["platform"].lower() == "slack"]
|
||||
assert len(slack_targets) == 1
|
||||
assert _target_mirror_eligible(job, slack_targets[0], global_mirror=True)
|
||||
|
||||
def test_explicit_target_not_eligible_under_global_flag(self):
|
||||
"""Global mirror_delivery must not write sessions into arbitrary
|
||||
@@ -167,8 +198,9 @@ class TestFallbackMirrorEndToEnd:
|
||||
|
||||
import gateway.mirror as mirror_mod
|
||||
|
||||
real_mirror = mirror_mod.mirror_to_session
|
||||
monkeypatch.setattr(mirror_mod, "mirror_to_session", fake_mirror)
|
||||
return {"send": send_calls, "mirror": mirror_calls}
|
||||
return {"send": send_calls, "mirror": mirror_calls, "real_mirror": real_mirror, "home": home}
|
||||
|
||||
def test_origin_fallback_job_mirrors_brief(self, slack_env):
|
||||
"""The field repro: managed cron, deliver=origin, no origin captured.
|
||||
@@ -191,6 +223,27 @@ class TestFallbackMirrorEndToEnd:
|
||||
assert len(slack_env["send"]) == 1
|
||||
assert len(slack_env["mirror"]) == 0
|
||||
|
||||
def test_managed_bare_platform_job_mirrors_brief(self, slack_env, monkeypatch):
|
||||
"""The managed-cron shape delivers into the session a home-channel reply resumes."""
|
||||
import gateway.mirror as mirror_mod
|
||||
from gateway.config import GatewayConfig, Platform
|
||||
from gateway.session import SessionSource, SessionStore
|
||||
|
||||
monkeypatch.setattr(mirror_mod, "mirror_to_session", slack_env["real_mirror"])
|
||||
store = SessionStore(slack_env["home"] / "sessions", GatewayConfig())
|
||||
source = SessionSource(platform=Platform.SLACK, chat_id="D0HOME", chat_type="dm")
|
||||
session = store.get_or_create_session(source)
|
||||
job = {"id": "h1", "name": "managed", "deliver": "slack", "origin": None}
|
||||
err = _deliver_result(job, "morning brief", adapters=None, loop=None)
|
||||
assert err is None
|
||||
assert len(slack_env["send"]) == 1
|
||||
reply_session = store.get_or_create_session(source)
|
||||
assert reply_session.session_id == session.session_id
|
||||
messages = store.load_transcript(reply_session.session_id)
|
||||
assert [(m["role"], m["content"]) for m in messages] == [
|
||||
("user", "[Cron delivery: managed]\nmorning brief")
|
||||
]
|
||||
|
||||
def test_explicit_target_with_attach_mirrors(self, slack_env):
|
||||
job = {
|
||||
"id": "j3", "name": "managed-dm", "deliver": "slack:D0USER7",
|
||||
|
||||
@@ -102,6 +102,7 @@ class TestConfigHomeChannelFallback:
|
||||
"platform": "discord",
|
||||
"chat_id": "1517373704248758474",
|
||||
"thread_id": None,
|
||||
"_resolved_from": "home",
|
||||
}]
|
||||
|
||||
|
||||
|
||||
@@ -214,6 +214,7 @@ class TestResolveDeliveryTarget:
|
||||
"platform": "discord",
|
||||
"chat_id": "home-parent",
|
||||
"thread_id": None,
|
||||
"_resolved_from": "home",
|
||||
}
|
||||
|
||||
def test_telegram_cron_thread_id_overrides_home_thread_id(self, monkeypatch):
|
||||
@@ -226,6 +227,7 @@ class TestResolveDeliveryTarget:
|
||||
"platform": "telegram",
|
||||
"chat_id": "-1001234567890",
|
||||
"thread_id": "42",
|
||||
"_resolved_from": "home",
|
||||
}
|
||||
|
||||
|
||||
@@ -312,6 +314,7 @@ class TestResolveDeliveryTarget:
|
||||
"platform": "telegram",
|
||||
"chat_id": "-4004",
|
||||
"thread_id": None,
|
||||
"_resolved_from": "home",
|
||||
}
|
||||
|
||||
|
||||
|
||||
@@ -985,7 +985,7 @@ Jobs run in a fresh session with no current-chat context, so prompts must be sel
|
||||
},
|
||||
"attach_to_session": {
|
||||
"type": "boolean",
|
||||
"description": "True = the job's delivery is CONTINUABLE — the user can reply and the agent has the brief in context (threads on thread-capable platforms, mirrored into the DM elsewhere). Use for conversational recurring jobs (briefings); leave unset for fire-and-forget alerts. Scope: the job's own conversation only — the origin chat, the home-channel fallback when deliver='origin' captured no origin (script-created jobs), or the job's single explicit platform:chat target (this flag is the only way to attach an explicit target). Broadcast targets are never attached; no effect when deliver='local'."
|
||||
"description": "True = the job's delivery is CONTINUABLE — the user can reply and the agent has the brief in context (threads on thread-capable platforms, mirrored into the DM elsewhere). Use for conversational recurring jobs (briefings); leave unset for fire-and-forget alerts. Scope: the job's own conversation only — the origin chat, the home-channel fallback when deliver='origin' captured no origin (script-created jobs), a user-written bare platform target (deliver='slack' — that platform's home channel), or the job's single explicit platform:chat target (this flag is the only way to attach an explicit target). Broadcast targets are never attached; no effect when deliver='local'."
|
||||
},
|
||||
},
|
||||
"required": ["action"]
|
||||
|
||||
@@ -603,9 +603,14 @@ Only the job's **own conversation** is ever touched:
|
||||
target a conversation. The global `mirror_delivery` flag alone never makes an
|
||||
explicitly-addressed chat continuable.
|
||||
|
||||
Broadcast / fan-out targets (`all`, bare-platform home channels) are never made
|
||||
continuable. The mirror is
|
||||
written as a labelled user turn (`[Cron delivery: <task name>]`), which keeps
|
||||
Broadcast expansions (`all`) are never made continuable. A user-written bare
|
||||
platform name (`deliver: slack`) addresses that platform's home channel
|
||||
deliberately and follows the same rules as the home-channel fallback above.
|
||||
After upgrading, existing `deliver: <platform>` jobs with `cron.mirror_delivery: true`
|
||||
can open a new thread per run on thread-capable platforms. Set `attach_to_session: false`
|
||||
on a job to opt out of this thread-per-run behaviour.
|
||||
|
||||
The mirror is written as a labelled user turn (`[Cron delivery: <task name>]`), which keeps
|
||||
the conversation history alternation-safe across all model providers.
|
||||
|
||||
#### Flat, in-channel continuation (Slack)
|
||||
|
||||
Reference in New Issue
Block a user