fix: tighten telegram docker-media salvage follow-ups
Follow-up on top of the helix4u #6392 cherry-pick: - reuse one helper for actionable Docker-local file-not-found errors across document/image/video/audio local-media send paths - include /outputs/... alongside /output/... in the container-local path hint - soften the gateway startup warning so it does not imply custom host-visible mounts are broken; the warning now targets the specific risky pattern of emitting container-local MEDIA paths without an explicit export mount - add focused regressions for /outputs/... and non-document media hint coverage This keeps the salvage aligned with the actual MEDIA delivery problem on current main while reducing false-positive operator messaging.
This commit is contained in:
@@ -1657,6 +1657,21 @@ class TelegramAdapter(BasePlatformAdapter):
|
|||||||
except Exception as exc:
|
except Exception as exc:
|
||||||
logger.error("Failed to write update response from callback: %s", exc)
|
logger.error("Failed to write update response from callback: %s", exc)
|
||||||
|
|
||||||
|
def _missing_media_path_error(self, label: str, path: str) -> str:
|
||||||
|
"""Build an actionable file-not-found error for gateway MEDIA delivery.
|
||||||
|
|
||||||
|
Paths like /workspace/... or /output/... often only exist inside the
|
||||||
|
Docker sandbox, while the gateway process runs on the host.
|
||||||
|
"""
|
||||||
|
error = f"{label} file not found: {path}"
|
||||||
|
if path.startswith(("/workspace/", "/output/", "/outputs/")):
|
||||||
|
error += (
|
||||||
|
" (path may only exist inside the Docker sandbox. "
|
||||||
|
"Bind-mount a host directory and emit the host-visible "
|
||||||
|
"path in MEDIA: for gateway file delivery.)"
|
||||||
|
)
|
||||||
|
return error
|
||||||
|
|
||||||
async def send_voice(
|
async def send_voice(
|
||||||
self,
|
self,
|
||||||
chat_id: str,
|
chat_id: str,
|
||||||
@@ -1673,7 +1688,7 @@ class TelegramAdapter(BasePlatformAdapter):
|
|||||||
try:
|
try:
|
||||||
import os
|
import os
|
||||||
if not os.path.exists(audio_path):
|
if not os.path.exists(audio_path):
|
||||||
return SendResult(success=False, error=f"Audio file not found: {audio_path}")
|
return SendResult(success=False, error=self._missing_media_path_error("Audio", audio_path))
|
||||||
|
|
||||||
with open(audio_path, "rb") as audio_file:
|
with open(audio_path, "rb") as audio_file:
|
||||||
# .ogg files -> send as voice (round playable bubble)
|
# .ogg files -> send as voice (round playable bubble)
|
||||||
@@ -1722,7 +1737,7 @@ class TelegramAdapter(BasePlatformAdapter):
|
|||||||
try:
|
try:
|
||||||
import os
|
import os
|
||||||
if not os.path.exists(image_path):
|
if not os.path.exists(image_path):
|
||||||
return SendResult(success=False, error=f"Image file not found: {image_path}")
|
return SendResult(success=False, error=self._missing_media_path_error("Image", image_path))
|
||||||
|
|
||||||
_thread = self._metadata_thread_id(metadata)
|
_thread = self._metadata_thread_id(metadata)
|
||||||
with open(image_path, "rb") as image_file:
|
with open(image_path, "rb") as image_file:
|
||||||
@@ -1759,14 +1774,7 @@ class TelegramAdapter(BasePlatformAdapter):
|
|||||||
|
|
||||||
try:
|
try:
|
||||||
if not os.path.exists(file_path):
|
if not os.path.exists(file_path):
|
||||||
error = f"File not found: {file_path}"
|
return SendResult(success=False, error=self._missing_media_path_error("File", file_path))
|
||||||
if file_path.startswith(("/workspace/", "/output/")):
|
|
||||||
error += (
|
|
||||||
" (path may only exist inside the Docker sandbox. "
|
|
||||||
"Bind-mount a host directory and emit the host-visible "
|
|
||||||
"path in MEDIA: for gateway file delivery.)"
|
|
||||||
)
|
|
||||||
return SendResult(success=False, error=error)
|
|
||||||
|
|
||||||
display_name = file_name or os.path.basename(file_path)
|
display_name = file_name or os.path.basename(file_path)
|
||||||
_thread = self._metadata_thread_id(metadata)
|
_thread = self._metadata_thread_id(metadata)
|
||||||
@@ -1800,7 +1808,7 @@ class TelegramAdapter(BasePlatformAdapter):
|
|||||||
|
|
||||||
try:
|
try:
|
||||||
if not os.path.exists(video_path):
|
if not os.path.exists(video_path):
|
||||||
return SendResult(success=False, error=f"Video file not found: {video_path}")
|
return SendResult(success=False, error=self._missing_media_path_error("Video", video_path))
|
||||||
|
|
||||||
_thread = self._metadata_thread_id(metadata)
|
_thread = self._metadata_thread_id(metadata)
|
||||||
with open(video_path, "rb") as f:
|
with open(video_path, "rb") as f:
|
||||||
|
|||||||
@@ -589,7 +589,7 @@ class GatewayRunner:
|
|||||||
def __init__(self, config: Optional[GatewayConfig] = None):
|
def __init__(self, config: Optional[GatewayConfig] = None):
|
||||||
self.config = config or load_gateway_config()
|
self.config = config or load_gateway_config()
|
||||||
self.adapters: Dict[Platform, BasePlatformAdapter] = {}
|
self.adapters: Dict[Platform, BasePlatformAdapter] = {}
|
||||||
self._warn_if_docker_media_delivery_is_likely_misconfigured()
|
self._warn_if_docker_media_delivery_is_risky()
|
||||||
|
|
||||||
# Load ephemeral config from config.yaml / env vars.
|
# Load ephemeral config from config.yaml / env vars.
|
||||||
# Both are injected at API-call time only and never persisted.
|
# Both are injected at API-call time only and never persisted.
|
||||||
@@ -696,12 +696,14 @@ class GatewayRunner:
|
|||||||
self._background_tasks: set = set()
|
self._background_tasks: set = set()
|
||||||
|
|
||||||
|
|
||||||
def _warn_if_docker_media_delivery_is_likely_misconfigured(self) -> None:
|
def _warn_if_docker_media_delivery_is_risky(self) -> None:
|
||||||
"""Warn when Docker-backed gateway setups lack an obvious output bind mount.
|
"""Warn when Docker-backed gateways lack an explicit export mount.
|
||||||
|
|
||||||
MEDIA delivery happens in the gateway process, so paths emitted by the model
|
MEDIA delivery happens in the gateway process, so paths emitted by the model
|
||||||
must be readable from the host. A plain container-local path like
|
must be readable from the host. A plain container-local path like
|
||||||
`/workspace/report.txt` often exists only inside Docker.
|
`/workspace/report.txt` or `/output/report.txt` often exists only inside
|
||||||
|
Docker, so users commonly need a dedicated export mount such as
|
||||||
|
`host-dir:/output`.
|
||||||
"""
|
"""
|
||||||
if os.getenv("TERMINAL_ENV", "").strip().lower() != "docker":
|
if os.getenv("TERMINAL_ENV", "").strip().lower() != "docker":
|
||||||
return
|
return
|
||||||
@@ -737,8 +739,8 @@ class GatewayRunner:
|
|||||||
logger.warning(
|
logger.warning(
|
||||||
"Docker backend is enabled for the messaging gateway but no explicit host-visible "
|
"Docker backend is enabled for the messaging gateway but no explicit host-visible "
|
||||||
"output mount (for example '/home/user/.hermes/cache/documents:/output') is configured. "
|
"output mount (for example '/home/user/.hermes/cache/documents:/output') is configured. "
|
||||||
"MEDIA file delivery can fail for files that only exist inside the container, such as "
|
"This is fine if the model already emits host-visible paths, but MEDIA file delivery can fail "
|
||||||
"'/workspace/...'."
|
"for container-local paths like '/workspace/...' or '/output/...'."
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -107,7 +107,6 @@ async def test_runner_allows_cron_only_mode_when_no_platforms_are_enabled(monkey
|
|||||||
assert state["gateway_state"] == "running"
|
assert state["gateway_state"] == "running"
|
||||||
|
|
||||||
|
|
||||||
<<<<<<< HEAD
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_runner_records_connected_platform_state_on_success(monkeypatch, tmp_path):
|
async def test_runner_records_connected_platform_state_on_success(monkeypatch, tmp_path):
|
||||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||||
|
|||||||
@@ -496,6 +496,19 @@ class TestSendDocument:
|
|||||||
assert "host-visible path" in result.error.lower()
|
assert "host-visible path" in result.error.lower()
|
||||||
connected_adapter._bot.send_document.assert_not_called()
|
connected_adapter._bot.send_document.assert_not_called()
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_send_document_outputs_path_has_docker_hint(self, connected_adapter):
|
||||||
|
"""Legacy /outputs paths also get the Docker hint."""
|
||||||
|
result = await connected_adapter.send_document(
|
||||||
|
chat_id="12345",
|
||||||
|
file_path="/outputs/report.txt",
|
||||||
|
)
|
||||||
|
|
||||||
|
assert result.success is False
|
||||||
|
assert "docker sandbox" in result.error.lower()
|
||||||
|
assert "host-visible path" in result.error.lower()
|
||||||
|
connected_adapter._bot.send_document.assert_not_called()
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_send_document_not_connected(self, adapter):
|
async def test_send_document_not_connected(self, adapter):
|
||||||
"""If bot is None, returns not connected error."""
|
"""If bot is None, returns not connected error."""
|
||||||
@@ -678,6 +691,17 @@ class TestSendVideo:
|
|||||||
assert result.success is False
|
assert result.success is False
|
||||||
assert "not found" in result.error.lower()
|
assert "not found" in result.error.lower()
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_send_video_workspace_path_has_docker_hint(self, connected_adapter):
|
||||||
|
result = await connected_adapter.send_video(
|
||||||
|
chat_id="12345",
|
||||||
|
video_path="/workspace/video.mp4",
|
||||||
|
)
|
||||||
|
|
||||||
|
assert result.success is False
|
||||||
|
assert "docker sandbox" in result.error.lower()
|
||||||
|
assert "host-visible path" in result.error.lower()
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_send_video_not_connected(self, adapter):
|
async def test_send_video_not_connected(self, adapter):
|
||||||
result = await adapter.send_video(
|
result = await adapter.send_video(
|
||||||
|
|||||||
Reference in New Issue
Block a user