mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-23 10:42:00 +00:00
fix(delivery): make cron output truncation configurable + adapter-aware
Gateway-level truncation (MAX_PLATFORM_OUTPUT=4000) was pre-empting adapter-side message splitting. Discord and Telegram both chunk long content natively in their send() via truncate_message(), but the delivery router truncated to 3800 chars + footer before the adapter ever saw the full payload — so long cron output was cut short instead of being delivered as multiple messages (issue #50126). Changes: - HERMES_DELIVERY_MAX_PLATFORM_OUTPUT env var makes the cap configurable (default 4000, backward compatible). Set to 0 to disable truncation. - TRUNCATED_VISIBLE (3800) removed — visible portion now derived dynamically from max_output minus the actual footer length. - New BasePlatformAdapter.splits_long_messages capability flag (default False). Adapters that chunk in send() set True; delivery skips truncation for them but still saves full output to disk as audit. - Flagged Discord and Telegram (both verified to chunk in send()). Fixes #50126
This commit is contained in:
ScotterMonk
Teknium
parent
eecb5b9dd1
commit
86e4521cb1
5 changed files with 288 additions and 10 deletions
|
|
@ -20,8 +20,34 @@ from hermes_cli.config import get_hermes_home
|
|||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
MAX_PLATFORM_OUTPUT = 4000
|
||||
TRUNCATED_VISIBLE = 3800
|
||||
# Default cap before gateway-level truncation of cron output for platform
|
||||
# delivery. Telegram's hard API limit is 4096; the 200-char headroom covers
|
||||
# the "full output saved to …" footer appended on truncation. Override via
|
||||
# the HERMES_DELIVERY_MAX_PLATFORM_OUTPUT env var. Adapters that split long
|
||||
# messages natively (BasePlatformAdapter.splits_long_messages) bypass this
|
||||
# entirely — the adapter chunks in its own send() and the full output is
|
||||
# preserved.
|
||||
_DEFAULT_MAX_PLATFORM_OUTPUT = 4000
|
||||
|
||||
|
||||
def _max_platform_output() -> int:
|
||||
"""Max chars before gateway-level truncation of cron output.
|
||||
|
||||
``HERMES_DELIVERY_MAX_PLATFORM_OUTPUT`` env var overrides the default
|
||||
(4000). Non-int or negative values fall back to the default with a
|
||||
warning.
|
||||
"""
|
||||
env = os.getenv("HERMES_DELIVERY_MAX_PLATFORM_OUTPUT")
|
||||
if env is not None:
|
||||
try:
|
||||
return max(0, int(env.strip()))
|
||||
except ValueError:
|
||||
logger.warning(
|
||||
"HERMES_DELIVERY_MAX_PLATFORM_OUTPUT=%r is not an int; "
|
||||
"using default %d",
|
||||
env, _DEFAULT_MAX_PLATFORM_OUTPUT,
|
||||
)
|
||||
return _DEFAULT_MAX_PLATFORM_OUTPUT
|
||||
|
||||
# Matches strings that are *only* a "silence" narration with optional markdown
|
||||
# wrappers. Covers: *(silent)*, _silent_, `silent`, ~silent~, (silent), silent,
|
||||
|
|
@ -316,14 +342,71 @@ class DeliveryRouter:
|
|||
if not target.chat_id:
|
||||
raise ValueError(f"No chat ID for {target.platform.value} delivery")
|
||||
|
||||
# Guard: truncate oversized cron output to stay within platform limits
|
||||
if len(content) > MAX_PLATFORM_OUTPUT:
|
||||
job_id = (metadata or {}).get("job_id", "unknown")
|
||||
saved_path = self._save_full_output(content, job_id)
|
||||
logger.info("Cron output truncated (%d chars) — full output: %s", len(content), saved_path)
|
||||
content = (
|
||||
content[:TRUNCATED_VISIBLE]
|
||||
+ f"\n\n... [truncated, full output saved to {saved_path}]"
|
||||
# Guard: handle oversized cron output.
|
||||
#
|
||||
# Two independent decisions:
|
||||
# 1. AUDIT SAVE — when content exceeds the audit threshold (4000
|
||||
# chars, the historical default), the full output is always
|
||||
# written to disk as a recoverable audit trail. This fires
|
||||
# regardless of truncation setting or adapter capability.
|
||||
# 2. TRUNCATION — for non-chunking adapters, content above
|
||||
# max_output is truncated with a footer pointing to the saved
|
||||
# file. Chunking-capable adapters (splits_long_messages=True)
|
||||
# receive the full payload and split natively in their send().
|
||||
# Setting HERMES_DELIVERY_MAX_PLATFORM_OUTPUT=0 disables
|
||||
# truncation entirely (the user takes responsibility for platform
|
||||
# API limits), but the audit save in step 1 still fires.
|
||||
max_output = _max_platform_output()
|
||||
job_id = (metadata or {}).get("job_id", "unknown")
|
||||
saved_path: Optional[Path] = None
|
||||
|
||||
# Step 1 — audit save (independent of truncation, best-effort).
|
||||
# The save is a side-effect audit trail, not essential to delivery.
|
||||
# If it fails (full disk, permissions), delivery proceeds — the
|
||||
# content reaches the adapter regardless. The truncation path's
|
||||
# fallback save below is NOT best-effort: the footer needs a valid
|
||||
# path, so a failure there is a real delivery problem.
|
||||
if len(content) > _DEFAULT_MAX_PLATFORM_OUTPUT:
|
||||
try:
|
||||
saved_path = self._save_full_output(content, job_id)
|
||||
except OSError as exc:
|
||||
logger.warning(
|
||||
"Audit save failed for cron output (%d chars, job=%s): %s — "
|
||||
"delivery proceeds without audit copy",
|
||||
len(content), job_id, exc,
|
||||
)
|
||||
|
||||
# Step 2 — truncation (only for non-chunking adapters).
|
||||
if max_output > 0 and len(content) > max_output:
|
||||
if adapter and getattr(adapter, "splits_long_messages", False):
|
||||
# Adapter chunks natively — deliver full payload.
|
||||
if saved_path:
|
||||
logger.info(
|
||||
"Cron output preserved for chunking adapter (%d chars) — "
|
||||
"full output saved to %s",
|
||||
len(content), saved_path,
|
||||
)
|
||||
else:
|
||||
# Non-chunking adapter — truncate with footer.
|
||||
if saved_path is None:
|
||||
# Content exceeded max_output but not the audit threshold
|
||||
# (e.g. HERMES_DELIVERY_MAX_PLATFORM_OUTPUT=200). Save
|
||||
# anyway since we're about to truncate.
|
||||
saved_path = self._save_full_output(content, job_id)
|
||||
footer = f"\n\n... [truncated, full output saved to {saved_path}]"
|
||||
visible = max(0, max_output - len(footer))
|
||||
logger.info(
|
||||
"Cron output truncated (%d chars) — full output: %s",
|
||||
len(content), saved_path,
|
||||
)
|
||||
content = content[:visible] + footer
|
||||
elif saved_path:
|
||||
# Truncation disabled (max_output=0) but content was large enough
|
||||
# to warrant an audit copy.
|
||||
logger.info(
|
||||
"Cron output delivered untruncated (%d chars, truncation "
|
||||
"disabled) — audit copy saved to %s",
|
||||
len(content), saved_path,
|
||||
)
|
||||
|
||||
# Substrate-level anti-loop guard: drop hallucinated "silence narration"
|
||||
|
|
|
|||
|
|
@ -2077,6 +2077,14 @@ class BasePlatformAdapter(ABC):
|
|||
# set this to False to stay correct-by-default.
|
||||
supports_async_delivery: bool = True
|
||||
|
||||
# Whether this adapter's ``send()`` splits long content into multiple
|
||||
# messages via ``truncate_message()``. When True, the delivery router
|
||||
# (gateway/delivery.py) skips gateway-level truncation and lets the
|
||||
# adapter chunk natively — preserving full output on platforms that
|
||||
# support multi-message delivery (Discord, Telegram, …). Default False
|
||||
# (conservative); adapters verified to chunk in ``send()`` set True.
|
||||
splits_long_messages: bool = False
|
||||
|
||||
# The command prefix users can always TYPE on this platform to reach
|
||||
# Hermes commands. Default "/" (most platforms deliver "/approve" etc.
|
||||
# as plain message text). Platforms where typing a leading "/" is
|
||||
|
|
|
|||
|
|
@ -733,6 +733,7 @@ class DiscordAdapter(BasePlatformAdapter):
|
|||
MAX_MESSAGE_LENGTH = 2000
|
||||
_SPLIT_THRESHOLD = 1900 # near the 2000-char split point
|
||||
supports_code_blocks = True # Discord markdown renders fenced code blocks natively
|
||||
splits_long_messages = True # send() chunks via truncate_message(MAX_MESSAGE_LENGTH)
|
||||
|
||||
# Auto-disconnect from voice channel after this many seconds of inactivity
|
||||
VOICE_TIMEOUT = 300
|
||||
|
|
|
|||
|
|
@ -417,6 +417,7 @@ class TelegramAdapter(BasePlatformAdapter):
|
|||
# Telegram message limits
|
||||
MAX_MESSAGE_LENGTH = 4096
|
||||
supports_code_blocks = True # Telegram MarkdownV2 renders fenced code blocks
|
||||
splits_long_messages = True # send() chunks via truncate_message(MAX_MESSAGE_LENGTH)
|
||||
# Bot API 10.1 Rich Messages cap the raw markdown/html text at 32,768
|
||||
# UTF-8 characters. Content above this is sent via the legacy chunking path.
|
||||
RICH_MESSAGE_MAX_CHARS = 32768
|
||||
|
|
|
|||
|
|
@ -281,3 +281,188 @@ async def test_platform_send_failure_raises_for_delivery_result(tmp_path, monkey
|
|||
|
||||
with pytest.raises(RuntimeError, match="route failed"):
|
||||
await router._deliver_to_platform(target, "hello", metadata={"telegram_reply_to_message_id": "9001"})
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Cron output truncation / adapter-aware chunking (issue #50126)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class ChunkingAdapter:
|
||||
"""Adapter that declares splits_long_messages=True (like Discord/Telegram)."""
|
||||
splits_long_messages = True
|
||||
|
||||
def __init__(self):
|
||||
self.calls = []
|
||||
|
||||
async def send(self, chat_id, content, metadata=None):
|
||||
self.calls.append({"chat_id": chat_id, "content": content, "metadata": metadata})
|
||||
return {"success": True}
|
||||
|
||||
|
||||
class NonChunkingAdapter:
|
||||
"""Adapter without splits_long_messages (default False — legacy behavior)."""
|
||||
|
||||
def __init__(self):
|
||||
self.calls = []
|
||||
|
||||
async def send(self, chat_id, content, metadata=None):
|
||||
self.calls.append({"chat_id": chat_id, "content": content, "metadata": metadata})
|
||||
return {"success": True}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_long_output_truncated_for_non_chunking_adapter(tmp_path, monkeypatch):
|
||||
"""Non-chunking adapters receive truncated content with a footer + file save."""
|
||||
monkeypatch.setattr("gateway.delivery.get_hermes_home", lambda: tmp_path)
|
||||
adapter = NonChunkingAdapter()
|
||||
router = DeliveryRouter(GatewayConfig(), adapters={Platform.DISCORD: adapter})
|
||||
target = DeliveryTarget.parse("discord:123")
|
||||
|
||||
long_content = "x" * 5000
|
||||
await router._deliver_to_platform(target, long_content, metadata={"job_id": "job1"})
|
||||
|
||||
delivered = adapter.calls[0]["content"]
|
||||
assert len(delivered) < 5000 # was truncated
|
||||
assert "truncated" in delivered.lower()
|
||||
assert "full output saved to" in delivered
|
||||
# Full output was saved to disk
|
||||
saved_files = list(tmp_path.glob("cron/output/job1_*.txt"))
|
||||
assert len(saved_files) == 1
|
||||
assert saved_files[0].read_text() == long_content
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_long_output_preserved_for_chunking_adapter(tmp_path, monkeypatch):
|
||||
"""Chunking adapters (splits_long_messages=True) receive the FULL content."""
|
||||
monkeypatch.setattr("gateway.delivery.get_hermes_home", lambda: tmp_path)
|
||||
adapter = ChunkingAdapter()
|
||||
router = DeliveryRouter(GatewayConfig(), adapters={Platform.DISCORD: adapter})
|
||||
target = DeliveryTarget.parse("discord:123")
|
||||
|
||||
long_content = "x" * 5000
|
||||
await router._deliver_to_platform(target, long_content, metadata={"job_id": "job2"})
|
||||
|
||||
delivered = adapter.calls[0]["content"]
|
||||
assert delivered == long_content # NOT truncated — adapter handles chunking
|
||||
assert "truncated" not in delivered.lower()
|
||||
# Full output still saved to disk as audit trail
|
||||
saved_files = list(tmp_path.glob("cron/output/job2_*.txt"))
|
||||
assert len(saved_files) == 1
|
||||
assert saved_files[0].read_text() == long_content
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_short_output_never_truncated(tmp_path, monkeypatch):
|
||||
"""Output under the limit passes through untouched for any adapter."""
|
||||
monkeypatch.setattr("gateway.delivery.get_hermes_home", lambda: tmp_path)
|
||||
adapter = NonChunkingAdapter()
|
||||
router = DeliveryRouter(GatewayConfig(), adapters={Platform.DISCORD: adapter})
|
||||
target = DeliveryTarget.parse("discord:123")
|
||||
|
||||
short_content = "x" * 100
|
||||
await router._deliver_to_platform(target, short_content, metadata={"job_id": "job3"})
|
||||
|
||||
assert adapter.calls[0]["content"] == short_content
|
||||
# Nothing saved to disk
|
||||
assert not list(tmp_path.glob("cron/output/*.txt"))
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_env_override_changes_truncation_threshold(tmp_path, monkeypatch):
|
||||
"""HERMES_DELIVERY_MAX_PLATFORM_OUTPUT env var overrides the default 4000."""
|
||||
monkeypatch.setattr("gateway.delivery.get_hermes_home", lambda: tmp_path)
|
||||
monkeypatch.setenv("HERMES_DELIVERY_MAX_PLATFORM_OUTPUT", "200")
|
||||
adapter = NonChunkingAdapter()
|
||||
router = DeliveryRouter(GatewayConfig(), adapters={Platform.DISCORD: adapter})
|
||||
target = DeliveryTarget.parse("discord:123")
|
||||
|
||||
content = "x" * 300 # over the env-override threshold of 200
|
||||
await router._deliver_to_platform(target, content, metadata={"job_id": "job4"})
|
||||
|
||||
delivered = adapter.calls[0]["content"]
|
||||
assert len(delivered) < 300 # truncated because env lowered the bar
|
||||
assert "truncated" in delivered.lower()
|
||||
# Audit file saved (truncation path always saves when it truncates)
|
||||
saved_files = list(tmp_path.glob("cron/output/job4_*.txt"))
|
||||
assert len(saved_files) == 1
|
||||
assert saved_files[0].read_text() == content
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_env_override_disable_truncation(tmp_path, monkeypatch):
|
||||
"""Setting HERMES_DELIVERY_MAX_PLATFORM_OUTPUT=0 disables truncation entirely."""
|
||||
monkeypatch.setattr("gateway.delivery.get_hermes_home", lambda: tmp_path)
|
||||
monkeypatch.setenv("HERMES_DELIVERY_MAX_PLATFORM_OUTPUT", "0")
|
||||
adapter = NonChunkingAdapter()
|
||||
router = DeliveryRouter(GatewayConfig(), adapters={Platform.DISCORD: adapter})
|
||||
target = DeliveryTarget.parse("discord:123")
|
||||
|
||||
content = "x" * 10000
|
||||
await router._deliver_to_platform(target, content, metadata={"job_id": "job5"})
|
||||
|
||||
# With max_output=0, truncation is disabled — even non-chunking adapters
|
||||
# receive the full content (they may error at the platform API level, but
|
||||
# that's the user's explicit choice).
|
||||
assert adapter.calls[0]["content"] == content
|
||||
# Audit file STILL saved — the audit threshold (4000) is independent of
|
||||
# the truncation setting. Content (10000) exceeds it.
|
||||
saved_files = list(tmp_path.glob("cron/output/job5_*.txt"))
|
||||
assert len(saved_files) == 1
|
||||
assert saved_files[0].read_text() == content
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_audit_save_failure_does_not_break_chunking_delivery(tmp_path, monkeypatch):
|
||||
"""If the audit save fails (disk full, permissions), chunking adapters
|
||||
still receive the full content — the save is best-effort."""
|
||||
monkeypatch.setattr("gateway.delivery.get_hermes_home", lambda: tmp_path)
|
||||
|
||||
adapter = ChunkingAdapter()
|
||||
router = DeliveryRouter(GatewayConfig(), adapters={Platform.DISCORD: adapter})
|
||||
target = DeliveryTarget.parse("discord:123")
|
||||
|
||||
long_content = "x" * 5000
|
||||
|
||||
call_count = {"n": 0}
|
||||
|
||||
def failing_save(content, job_id):
|
||||
call_count["n"] += 1
|
||||
raise OSError("No space left on device")
|
||||
|
||||
monkeypatch.setattr(router, "_save_full_output", failing_save)
|
||||
|
||||
# Should NOT raise — audit failure is caught
|
||||
await router._deliver_to_platform(target, long_content, metadata={"job_id": "job6"})
|
||||
|
||||
# Adapter still got the full content
|
||||
assert adapter.calls[0]["content"] == long_content
|
||||
# Save was attempted
|
||||
assert call_count["n"] == 1
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_audit_save_failure_does_not_break_non_chunking_delivery(tmp_path, monkeypatch):
|
||||
"""If the audit save fails AND truncation is needed, the fallback save
|
||||
in Step 2 is NOT caught — the footer needs a valid path, so this is a
|
||||
real failure. But if content exceeds the audit threshold AND truncation
|
||||
is disabled (max_output=0), the caught Step 1 failure lets delivery
|
||||
proceed."""
|
||||
monkeypatch.setattr("gateway.delivery.get_hermes_home", lambda: tmp_path)
|
||||
monkeypatch.setenv("HERMES_DELIVERY_MAX_PLATFORM_OUTPUT", "0")
|
||||
|
||||
adapter = NonChunkingAdapter()
|
||||
router = DeliveryRouter(GatewayConfig(), adapters={Platform.DISCORD: adapter})
|
||||
target = DeliveryTarget.parse("discord:123")
|
||||
|
||||
long_content = "x" * 5000
|
||||
|
||||
def failing_save(content, job_id):
|
||||
raise OSError("No space left on device")
|
||||
|
||||
monkeypatch.setattr(router, "_save_full_output", failing_save)
|
||||
|
||||
# max_output=0 → no truncation → Step 1 failure is caught → delivery proceeds
|
||||
await router._deliver_to_platform(target, long_content, metadata={"job_id": "job7"})
|
||||
|
||||
# Non-chunking adapter still got the full content (truncation disabled)
|
||||
assert adapter.calls[0]["content"] == long_content
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue