refactor: salvage adapter and CLI cleanup from PR #939

agent/display.py

@@ -68,7 +68,7 @@ def _oneline(text: str) -> str:
     return " ".join(text.split())
 
 
-def build_tool_preview(tool_name: str, args: dict, max_len: int = 40) -> str:
+def build_tool_preview(tool_name: str, args: dict, max_len: int = 40) -> str | None:
     """Build a short preview of a tool call's primary argument for display."""
     if not args:
         return None

gateway/platforms/slack.py

@@ -260,6 +260,30 @@ def _resolve_thread_ts(
                 return metadata["thread_ts"]
         return reply_to
 
+    async def _upload_file(
+        self,
+        chat_id: str,
+        file_path: str,
+        caption: Optional[str] = None,
+        reply_to: Optional[str] = None,
+        metadata: Optional[Dict[str, Any]] = None,
+    ) -> SendResult:
+        """Upload a local file to Slack."""
+        if not self._app:
+            return SendResult(success=False, error="Not connected")
+
+        if not os.path.exists(file_path):
+            raise FileNotFoundError(f"File not found: {file_path}")
+
+        result = await self._app.client.files_upload_v2(
+            channel=chat_id,
+            file=file_path,
+            filename=os.path.basename(file_path),
+            initial_comment=caption or "",
+            thread_ts=self._resolve_thread_ts(reply_to, metadata),
+        )
+        return SendResult(success=True, raw_response=result)
+
     # ----- Markdown → mrkdwn conversion -----
 
     def format_message(self, content: str) -> str:
@@ -417,23 +441,10 @@ async def send_image_file(
         metadata: Optional[Dict[str, Any]] = None,
     ) -> SendResult:
         """Send a local image file to Slack by uploading it."""
-        if not self._app:
-            return SendResult(success=False, error="Not connected")
-
         try:
-            import os
-            if not os.path.exists(image_path):
-                return SendResult(success=False, error=f"Image file not found: {image_path}")
-
-            result = await self._app.client.files_upload_v2(
-                channel=chat_id,
-                file=image_path,
-                filename=os.path.basename(image_path),
-                initial_comment=caption or "",
-                thread_ts=self._resolve_thread_ts(reply_to, metadata),
-            )
-            return SendResult(success=True, raw_response=result)
-
+            return await self._upload_file(chat_id, image_path, caption, reply_to, metadata)
+        except FileNotFoundError:
+            return SendResult(success=False, error=f"Image file not found: {image_path}")
         except Exception as e:  # pragma: no cover - defensive logging
             logger.error(
                 "[%s] Failed to send local Slack image %s: %s",
@@ -497,19 +508,10 @@ async def send_voice(
         metadata: Optional[Dict[str, Any]] = None,
     ) -> SendResult:
         """Send an audio file to Slack."""
-        if not self._app:
-            return SendResult(success=False, error="Not connected")
-
         try:
-            result = await self._app.client.files_upload_v2(
-                channel=chat_id,
-                file=audio_path,
-                filename=os.path.basename(audio_path),
-                initial_comment=caption or "",
-                thread_ts=self._resolve_thread_ts(reply_to, metadata),
-            )
-            return SendResult(success=True, raw_response=result)
-
+            return await self._upload_file(chat_id, audio_path, caption, reply_to, metadata)
+        except FileNotFoundError:
+            return SendResult(success=False, error=f"Audio file not found: {audio_path}")
         except Exception as e:  # pragma: no cover - defensive logging
             logger.error(
                 "[Slack] Failed to send audio file %s: %s",

hermes_cli/main.py

@@ -69,13 +69,33 @@
 os.environ.setdefault("MSWEA_SILENT_STARTUP", "1")
 
 import logging
+import time as _time
+from datetime import datetime
 
 from hermes_cli import __version__, __release_date__
 from hermes_constants import OPENROUTER_BASE_URL
 
 logger = logging.getLogger(__name__)
 
 
+def _relative_time(ts) -> str:
+    """Format a timestamp as relative time (e.g., '2h ago', 'yesterday')."""
+    if not ts:
+        return "?"
+    delta = _time.time() - ts
+    if delta < 60:
+        return "just now"
+    if delta < 3600:
+        return f"{int(delta / 60)}m ago"
+    if delta < 86400:
+        return f"{int(delta / 3600)}h ago"
+    if delta < 172800:
+        return "yesterday"
+    if delta < 604800:
+        return f"{int(delta / 86400)}d ago"
+    return datetime.fromtimestamp(ts).strftime("%Y-%m-%d")
+
+
 def _has_any_provider_configured() -> bool:
     """Check if at least one inference provider is usable."""
     from hermes_cli.config import get_env_path, get_hermes_home
@@ -140,28 +160,9 @@ def _session_browse_picker(sessions: list) -> Optional[str]:
     # Try curses-based picker first
     try:
         import curses
-        import time as _time
-        from datetime import datetime
 
         result_holder = [None]
 
-        def _relative_time(ts):
-            if not ts:
-                return "?"
-            delta = _time.time() - ts
-            if delta < 60:
-                return "just now"
-            elif delta < 3600:
-                return f"{int(delta / 60)}m ago"
-            elif delta < 86400:
-                return f"{int(delta / 3600)}h ago"
-            elif delta < 172800:
-                return "yesterday"
-            elif delta < 604800:
-                return f"{int(delta / 86400)}d ago"
-            else:
-                return datetime.fromtimestamp(ts).strftime("%Y-%m-%d")
-
         def _format_row(s, max_x):
             """Format a session row for display."""
             title = (s.get("title") or "").strip()
@@ -352,34 +353,14 @@ def _curses_browse(stdscr):
         pass
 
     # Fallback: numbered list (Windows without curses, etc.)
-    import time as _time
-    from datetime import datetime
-
-    def _relative_time_fb(ts):
-        if not ts:
-            return "?"
-        delta = _time.time() - ts
-        if delta < 60:
-            return "just now"
-        elif delta < 3600:
-            return f"{int(delta / 60)}m ago"
-        elif delta < 86400:
-            return f"{int(delta / 3600)}h ago"
-        elif delta < 172800:
-            return "yesterday"
-        elif delta < 604800:
-            return f"{int(delta / 86400)}d ago"
-        else:
-            return datetime.fromtimestamp(ts).strftime("%Y-%m-%d")
-
     print("\n  Browse sessions  (enter number to resume, q to cancel)\n")
     for i, s in enumerate(sessions):
         title = (s.get("title") or "").strip()
         preview = (s.get("preview") or "").strip()
         label = title or preview or s["id"]
         if len(label) > 50:
             label = label[:47] + "..."
-        last_active = _relative_time_fb(s.get("last_active"))
+        last_active = _relative_time(s.get("last_active"))
         src = s.get("source", "")[:6]
         print(f"  {i + 1:>3}. {label:<50}  {last_active:<10}  {src}")
 
@@ -2846,30 +2827,6 @@ def cmd_sessions(args):
             if not sessions:
                 print("No sessions found.")
                 return
-            from datetime import datetime
-            import time as _time
-
-            def _relative_time(ts):
-                """Format a timestamp as relative time (e.g., '2h ago', 'yesterday')."""
-                if not ts:
-                    return "?"
-                delta = _time.time() - ts
-                if delta < 60:
-                    return "just now"
-                elif delta < 3600:
-                    mins = int(delta / 60)
-                    return f"{mins}m ago"
-                elif delta < 86400:
-                    hours = int(delta / 3600)
-                    return f"{hours}h ago"
-                elif delta < 172800:
-                    return "yesterday"
-                elif delta < 604800:
-                    days = int(delta / 86400)
-                    return f"{days}d ago"
-                else:
-                    return datetime.fromtimestamp(ts).strftime("%Y-%m-%d")
-
             has_titles = any(s.get("title") for s in sessions)
             if has_titles:
                 print(f"{'Title':<22} {'Preview':<40} {'Last Active':<13} {'ID'}")

hermes_state.py

@@ -267,8 +267,6 @@ def sanitize_title(title: Optional[str]) -> Optional[str]:
         if not title:
             return None
 
-        import re
-
         # Remove ASCII control characters (0x00-0x1F, 0x7F) but keep
         # whitespace chars (\t=0x09, \n=0x0A, \r=0x0D) so they can be
         # normalized to spaces by the whitespace collapsing step below
@@ -373,7 +371,6 @@ def get_next_title_in_lineage(self, base_title: str) -> str:
         Strips any existing " #N" suffix to find the base name, then finds
         the highest existing number and increments.
         """
-        import re
         # Strip existing #N suffix to find the true base
         match = re.match(r'^(.*?) #(\d+)$', base_title)
         if match:

tests/gateway/test_send_image_file.py

@@ -297,7 +297,9 @@ class TestScreenshotCleanup:
     def test_cleanup_removes_old_screenshots(self, tmp_path):
         """_cleanup_old_screenshots should remove files older than max_age_hours."""
         import time
-        from tools.browser_tool import _cleanup_old_screenshots
+        from tools.browser_tool import _cleanup_old_screenshots, _last_screenshot_cleanup_by_dir
+
+        _last_screenshot_cleanup_by_dir.clear()
 
         # Create a "fresh" file
         fresh = tmp_path / "browser_screenshot_fresh.png"
@@ -314,10 +316,32 @@ def test_cleanup_removes_old_screenshots(self, tmp_path):
         assert fresh.exists(), "Fresh screenshot should not be removed"
         assert not old.exists(), "Old screenshot should be removed"
 
+    def test_cleanup_is_throttled_per_directory(self, tmp_path):
+        import time
+        from tools.browser_tool import _cleanup_old_screenshots, _last_screenshot_cleanup_by_dir
+
+        _last_screenshot_cleanup_by_dir.clear()
+
+        old = tmp_path / "browser_screenshot_old.png"
+        old.write_bytes(b"old")
+        old_time = time.time() - (25 * 3600)
+        os.utime(str(old), (old_time, old_time))
+
+        _cleanup_old_screenshots(tmp_path, max_age_hours=24)
+        assert not old.exists()
+
+        old.write_bytes(b"old-again")
+        os.utime(str(old), (old_time, old_time))
+        _cleanup_old_screenshots(tmp_path, max_age_hours=24)
+
+        assert old.exists(), "Repeated cleanup should be skipped while throttled"
+
     def test_cleanup_ignores_non_screenshot_files(self, tmp_path):
         """Only files matching browser_screenshot_*.png should be cleaned."""
         import time
-        from tools.browser_tool import _cleanup_old_screenshots
+        from tools.browser_tool import _cleanup_old_screenshots, _last_screenshot_cleanup_by_dir
+
+        _last_screenshot_cleanup_by_dir.clear()
 
         other_file = tmp_path / "important_data.txt"
         other_file.write_bytes(b"keep me")
@@ -330,11 +354,13 @@ def test_cleanup_ignores_non_screenshot_files(self, tmp_path):
 
     def test_cleanup_handles_empty_dir(self, tmp_path):
         """Cleanup should not fail on empty directory."""
-        from tools.browser_tool import _cleanup_old_screenshots
+        from tools.browser_tool import _cleanup_old_screenshots, _last_screenshot_cleanup_by_dir
+        _last_screenshot_cleanup_by_dir.clear()
         _cleanup_old_screenshots(tmp_path, max_age_hours=24)  # Should not raise
 
     def test_cleanup_handles_nonexistent_dir(self):
         """Cleanup should not fail if directory doesn't exist."""
         from pathlib import Path
-        from tools.browser_tool import _cleanup_old_screenshots
+        from tools.browser_tool import _cleanup_old_screenshots, _last_screenshot_cleanup_by_dir
+        _last_screenshot_cleanup_by_dir.clear()
         _cleanup_old_screenshots(Path("/nonexistent/dir"), max_age_hours=24)  # Should not raise

tools/browser_tool.py

@@ -67,6 +67,12 @@
 
 logger = logging.getLogger(__name__)
 
+# Standard PATH entries for environments with minimal PATH (e.g. systemd services)
+_SANE_PATH = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
+
+# Throttle screenshot cleanup to avoid repeated full directory scans.
+_last_screenshot_cleanup_by_dir: dict[str, float] = {}
+
 # ============================================================================
 # Configuration
 # ============================================================================
@@ -846,7 +852,6 @@ def _run_browser_command(
 
         browser_env = {**os.environ}
         # Ensure PATH includes standard dirs (systemd services may have minimal PATH)
-        _SANE_PATH = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
         if "/usr/bin" not in browser_env.get("PATH", "").split(":"):
             browser_env["PATH"] = f"{browser_env.get('PATH', '')}:{_SANE_PATH}"
         browser_env["AGENT_BROWSER_SOCKET_DIR"] = task_socket_dir
@@ -1578,8 +1583,17 @@ def browser_vision(question: str, annotate: bool = False, task_id: Optional[str]
 
 
 def _cleanup_old_screenshots(screenshots_dir, max_age_hours=24):
-    """Remove browser screenshots older than max_age_hours to prevent disk bloat."""
-    import time
+    """Remove browser screenshots older than max_age_hours to prevent disk bloat.
+
+    Throttled to run at most once per hour per directory to avoid repeated
+    scans on screenshot-heavy workflows.
+    """
+    key = str(screenshots_dir)
+    now = time.time()
+    if now - _last_screenshot_cleanup_by_dir.get(key, 0.0) < 3600:
+        return
+    _last_screenshot_cleanup_by_dir[key] = now
+
     try:
         cutoff = time.time() - (max_age_hours * 3600)
         for f in screenshots_dir.glob("browser_screenshot_*.png"):