fix(gateway): rewind MarkdownV2-escaped pipe tables in cron Telegram deliveries

.github/workflows/docker.yml

@@ -26,8 +26,14 @@ env:
 jobs:
   # Build, test, and optionally push the amd64 image.
   build-amd64:
-    # Only run on the upstream repository, not on forks
-    if: github.repository == 'NousResearch/hermes-agent'
+    # Skip PR builds from forks: GITHUB_TOKEN for fork PRs lacks `packages: write`,
+    # so the cache-to step (write to ghcr.io) fails with "installation not allowed
+    # to Write organization package". Upstream PRs run the full pipeline; forks
+    # get gate-level coverage from ci.yml + lint.yml + typecheck.yml + tests.yml.
+    # Use head.repo.full_name instead of github.repository because this workflow
+    # is called as a reusable workflow from ci.yml, where github.repository is
+    # always the upstream repo (NousResearch/hermes-agent).
+    if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == 'NousResearch/hermes-agent'
     runs-on: ubuntu-latest
     timeout-minutes: 45
     outputs:
@@ -146,7 +152,14 @@ jobs:
   # Build, test, and optionally push the arm64 image.
   # ---------------------------------------------------------------------------
   build-arm64:
-    if: github.repository == 'NousResearch/hermes-agent'
+    # Skip PR builds from forks: GITHUB_TOKEN for fork PRs lacks `packages: write`,
+    # so the cache-to step (write to ghcr.io) fails with "installation not allowed
+    # to Write organization package". Upstream PRs run the full pipeline; forks
+    # get gate-level coverage from ci.yml + lint.yml + typecheck.yml + tests.yml.
+    # Use head.repo.full_name instead of github.repository because this workflow
+    # is called as a reusable workflow from ci.yml, where github.repository is
+    # always the upstream repo (NousResearch/hermes-agent).
+    if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == 'NousResearch/hermes-agent'
     runs-on: ubuntu-24.04-arm
     timeout-minutes: 45
     outputs:

PR_BODY_53175.md

@@ -0,0 +1,106 @@
+## Summary
+
+Fixes gateway event loop zombie state (#53175) where `agent.close()` and
+`shutdown_memory_provider()` blocked the event loop during subprocess
+teardown, causing silent message processing death (16 crashes in ~30h).
+
+Replaces 7 scattered synchronous `_cleanup_agent_resources()` call sites
+with one centralized `_cleanup_agent_async()` that offloads all blocking
+I/O to a thread pool executor with per-context timeouts and structured
+logging.
+
+## Problem
+
+Issue #53175: the gateway enters a zombie state where the process stays
+alive, platform status shows "connected," but the event loop silently
+stops processing inbound messages. Root cause: `_cleanup_agent_resources()`
+runs synchronous blocking code (`agent.close()`, `shutdown_memory_provider()`)
+inside async handlers. Three bare `except Exception: pass` blocks made the
+blockage invisible.
+
+Each zombie crash was preceded by long responses (>100s) + session
+reset (`/new`), exactly when agent cleanup is triggered on a loaded LLM
+client with active subprocesses and network connections.
+
+## Solution
+
+### New `_CleanupContext` enum
+Categorizes all 7 cleanup triggers for selective timeout & logging:
+
+| Context | Default timeout | Trigger |
+|---|---|---|
+| `SHUTDOWN` | 30s | Gateway stop/restart |
+| `SESSION_EXPIRY` | 30s | 5-min watchdog finalization |
+| `SESSION_HYGIENE` | 30s | Post-auto-compress eviction |
+| `IDLE_CACHE_EVICTION` | 30s | Shutdown idle cached agents |
+| `BACKGROUND_TASK` | 15s | After executor-run background task |
+| `CACHE_FALLBACK` | 10s | `_release_evicted_agent_soft` fallback |
+
+### `_cleanup_agent_async(agent, context, session_key)`
+Centralized async cleanup:
+```python
+await asyncio.wait_for(
+    self._run_in_executor_with_context(
+        self._cleanup_agent_resources, agent
+    ),
+    timeout=self._get_cleanup_timeout(context),
+)
+```
+- Runs blocking ops in thread pool → event loop never stalls
+- Timeout per context (configurable) → stuck agent can't take down gateway
+- Structured logging with `session_id` + `context` → debuggable
+- `TimeoutError` → warning + continue (preserves liveness)
+- `Exception` → warning + continue (replaces silent `pass`)
+
+### `_cleanup_agent_resources()` (updated)
+Replaced 3 silent `except Exception: pass` with `logger.warning()` that
+includes session_id and operation name:
+- `shutdown_memory_provider()` failure → logged
+- `agent.close()` failure → logged
+- `cleanup_stale_async_clients()` failure → logged
+
+### 7 call sites migrated
+| # | Call site | Old | New |
+|---|---|---|---|
+| 1 | `_finalize_shutdown_agents()` | sync `_cleanup_agent_resources` | `_cleanup_agent_async` + threadpool (SHUTDOWN) |
+| 2 | `_session_expiry_watcher()` | sync `_cleanup_agent_resources` | `await _cleanup_agent_async` (SESSION_EXPIRY) |
+| 3 | Shutdown idle cache cleanup | sync `_cleanup_agent_resources` | `await _cleanup_agent_async` (IDLE_CACHE_EVICTION) |
+| 4 | Session hygiene | sync `_cleanup_agent_resources` | `await _cleanup_agent_async` (SESSION_HYGIENE) |
+| 5 | Background task executor | sync `_cleanup_agent_resources` | kept sync (already in executor thread) + comment |
+| 6 | `_release_evicted_agent_soft()` fallback | sync `_cleanup_agent_resources` | kept sync (already in daemon thread) + comment |
+| 7 | Cross-process cache invalidation | sync `_cleanup_agent_resources` | kept sync (already in daemon thread) + comment |
+
+### `_finalize_shutdown_agents` sync fallback
+When `_gateway_loop` is None (tests), runs cleanup synchronously so unit
+tests can verify `close()` was called immediately. Simplified the previous
+`asyncio.get_running_loop()` fallback chain.
+
+### Optional config
+```yaml
+gateway:
+  cleanup_timeouts:
+    shutdown: 30.0
+    session_expiry: 30.0
+    session_hygiene: 30.0
+    idle_cache: 30.0
+    background_task: 15.0
+    cache_fallback: 10.0
+```
+
+## Files changed
+
+| File | Changes |
+|---|---|
+| `gateway/run.py` | +191/-43 — `_CleanupContext` enum, `_cleanup_agent_async()`, `_get_cleanup_timeout()`, updated `_cleanup_agent_resources()`, migrated 7 call sites |
+| `tests/gateway/test_shutdown_cache_cleanup.py` | +44/-43 — updated for async cleanup pattern, 8/8 passing |
+| `.github/workflows/docker-publish.yml` | +13/-3 — skip Docker build on fork PRs (no `packages: write`) |
+
+## Commits
+
+1. `5724c4e` — fix(gateway): centralize agent cleanup pipeline with executor offload (#53175)
+2. `deedb4b` — test(gateway): update shutdown cache cleanup tests for async cleanup pattern
+3. `cd63145` — fix(gateway): ensure _finalize_shutdown_agents uses safe_schedule_threadsafe with sync fallback
+4. `fbf7e44` — fix(gateway): ensure _finalize_shutdown_agents runs sync cleanup without _gateway_loop
+5. `522ad29` — ci(docker-publish): skip arm64/amd64 build jobs on fork PRs
+
+Closes #53175

gateway/delivery.py

@@ -40,6 +40,107 @@
 )
 
 
+# Matches a MarkdownV2-escaped pipe (``\|``) outside of formatting markers and
+# inline code spans.  Used to detect and rewind overly-aggressive pipe-table
+# escaping in cron-delivered agent outputs (#53632).
+_MARKDOWNV2_TABLE_BAR = re.compile(
+    r'(?<!\\)(?<![`*_~])\|(?![`*_~])'
+)
+# Backslash-escape the pipe literal sequence, so the two-char token ``\|``
+# (rather than just ``|``) is what we count and replace.
+_MARKDOWNV2_ESCAPED_TABLE_BAR = re.compile(r'\\\|')
+
+
+def _looks_like_cron_markdownv2_table(content: Optional[str]) -> bool:
+    r"""Return True if ``content`` looks like a MarkdownV2-escaped pipe table.
+
+    A model that follows Telegram Bot API MarkdownV2 syntax emits table rows
+    with ``\|`` instead of ``|``.  Telegram's MarkdownV2 parser renders the
+    backslash-pipe verbatim as ``\|`` - which is what cron recipients see in
+    their broken-tables bug report (#53632).  Detection rules:
+
+    * At least one line in the content carries ``>=3`` ``\|`` tokens AND
+      the line matches a ``header / separator / data`` row shape (bars
+      bracketing text segments between them).
+    * At least 6 total escaped pipes across the whole content (a real pipe
+      table has 3 rows * 2 pipes/row minimum: header + separator + body).
+
+    Conservative: returns False for prose that merely contains ``\|`` as an
+    escape artifact (e.g. arithmetic ``a \| b``).
+    """
+    if not content:
+        return False
+    total_escaped = sum(
+        len(_MARKDOWNV2_ESCAPED_TABLE_BAR.findall(line))
+        for line in content.splitlines()
+    )
+    if total_escaped < 6:
+        return False
+    pipe_table_like_lines = 0
+    for line in content.splitlines():
+        if len(_MARKDOWNV2_ESCAPED_TABLE_BAR.findall(line)) >= 3:
+            # A pipe-table row has text on both sides of at least one bar, e.g.
+            # ``\| Date \| Event \|``.  Re-split on the bars to confirm.
+            re_split = _MARKDOWNV2_ESCAPED_TABLE_BAR.split(line.strip())
+            non_empty = [cell for cell in re_split if cell.strip()]
+            if len(non_empty) >= 3:
+                pipe_table_like_lines += 1
+    return pipe_table_like_lines >= 2  # at least header + one data row
+
+
+def _unescape_markdownv2_tables(content: str) -> str:
+    r"""Rewind ``\|`` to ``|`` inside lines that look like pipe-table rows.
+
+    LLM prompts that instruct ``"use Telegram MarkdownV2 syntax"`` cause the
+    model to emit table bars already-escaped: ``\| Date \| Event \|``.
+    Passing that string straight through Telegram's MarkdownV2 renderer shows
+    ``\|`` as literal text instead of rendering a native table (#53632).
+
+    This function rewinds the over-escape **only** on lines that match a
+    pipe-table row shape (``>=3`` escaped bars with cells on both sides).
+    Lines that don't match - code, prose, arithmetic - are returned
+    untouched.  The downstream adapter's normal MarkdownV2 escaping then
+    re-applies the ``\|`` once (the documented behavior) and Telegram
+    renders the table natively.
+
+    Code-block isolation is intentionally NOT done here: the only caller
+    hooks cron-delivered agent outputs whose format pipeline runs *after*
+    code-block delimiters have already been extracted by the adapter layer.
+    If a cron message contains ``\|`` inside a code fence (rare; the model
+    normally doesn't format code in MarkdownV2), the over-escape is harmless
+    - Telegram shows ``\|`` inside the code block, which is the same
+    behavior as today.
+    """
+    if not content:
+        return content
+    # Drive the unescaper off the same detection predicate as the gate
+    # uses (``_deliver_to_platform`` in the DeliveryRouter).  This keeps
+    # the two paths in lock-step — every input that the gate would have
+    # rewound gets rewound here, and only those inputs.
+    if not _looks_like_cron_markdownv2_table(content):
+        return content
+    out_lines = []
+    changed = False
+    for line in content.splitlines():
+        escaped = _MARKDOWNV2_ESCAPED_TABLE_BAR.findall(line)
+        if len(escaped) >= 3:
+            re_split = _MARKDOWNV2_ESCAPED_TABLE_BAR.split(line.strip())
+            non_empty = [cell for cell in re_split if cell.strip()]
+            if len(non_empty) >= 3:
+                # Pipe-table row shape — restore plain pipes so adapter's
+                # MarkdownV2 escape re-applies once and Telegram renders
+                # a native table.
+                new_line = line.replace('\\|', '|')
+                if new_line != line:
+                    changed = True
+                out_lines.append(new_line)
+                continue
+        out_lines.append(line)
+    if not changed:
+        return content
+    return '\n'.join(out_lines)
+
+
 def _is_silence_narration(content: Optional[str]) -> bool:
     """Return True when ``content`` is *only* a silence-narration token.
 
@@ -392,6 +493,35 @@ async def _deliver_to_platform(
                 "delivered": False,
             }
 
+        # Guard: rewind MarkdownV2-escaped pipe tables in cron-delivered agent
+        # outputs (#53632). Some prompts instruct models to use Telegram
+        # MarkdownV2 syntax directly, which causes the LLM to emit table bars
+        # already-escaped (``\|``). Telegram renders those literal ``\|`` as
+        # plain text - the recipient sees broken tables. This guard restores
+        # the plain ``|`` so the adapter's normal MarkdownV2 escape re-applies
+        # once and Telegram renders the table natively.
+        #
+        # Scope: ONLY cron-routed deliveries (job_id metadata set) for the
+        # Telegram platform. Other paths (interactive streaming final, other
+        # platforms, non-cron tool sends) are unchanged so the rewinding can't
+        # collide with `_send_telegram` family PRs (#46118 / #46952 / #47190).
+        is_cron_path = bool((metadata or {}).get("job_id"))
+        if (
+            target.platform == Platform.TELEGRAM
+            and is_cron_path
+            and _looks_like_cron_markdownv2_table(content)
+        ):
+            pre_len = len(content)
+            content = _unescape_markdownv2_tables(content)
+            if len(content) != pre_len:
+                logger.debug(
+                    "Rewound MarkdownV2-escaped pipe table in cron output "
+                    "(%d -> %d chars) for telegram chat=%s",
+                    pre_len,
+                    len(content),
+                    target.chat_id,
+                )
+
         send_metadata = dict(metadata or {})
         is_named_telegram_private_topic = False
         named_telegram_private_topic_name: Optional[str] = None