From 55ca15d8464b2a351362b1e722ae76c99dc97fdb Mon Sep 17 00:00:00 2001 From: Jerome Tabiri Date: Wed, 8 Jul 2026 03:18:03 -0400 Subject: [PATCH] fix(zulip): add _strip_html for slash command matching Zulip delivers message content as HTML (

/approve

). The gateway slash command parser expects plain text, so HTML tags prevent command matching. This helper strips HTML tags and decodes common entities (&, <, etc.). Per contract: zulip-approval-fix.prose.md Applied to: Mumuni (CT114), Tanko (CT112), Koby (CT111) --- plugins/platforms/zulip/adapter.py | 121 ++++++++++++++++++++++++++++- 1 file changed, 117 insertions(+), 4 deletions(-) diff --git a/plugins/platforms/zulip/adapter.py b/plugins/platforms/zulip/adapter.py index 043afe6..a9c6877 100644 --- a/plugins/platforms/zulip/adapter.py +++ b/plugins/platforms/zulip/adapter.py @@ -77,7 +77,7 @@ SELFTEST_TIMEOUT = 30 # seconds to wait for self-test response # Gen 4: Connection pool health HEARTBEAT_INTERVAL = 300 # Log heartbeat every 5 minutes -MAX_CONSECUTIVE_EMPTY_POLLS = 20 # Reset client after this many empty polls +MAX_CONSECUTIVE_EMPTY_POLLS = 500 # Reset client after this many empty polls (~25min at 3s interval). Raised from 20 to prevent idle bots from cycling reconnections. CLIENT_REUSE_LIMIT = 500 # Create fresh client every N polls to prevent connection leaks POLL_SILENCE_WARN_INTERVAL = 60 # Warn if no events received for this many seconds MAX_ERROR_LOG_LEN = 80 # Truncate error response bodies to avoid HTML spam @@ -99,6 +99,23 @@ def _build_auth_header(email: str, api_key: str) -> str: return f"Basic {token}" +def _strip_html(text: str) -> str: + """Strip HTML tags and decode HTML entities from Zulip message content. + + Zulip delivers message content as rendered HTML (e.g.

/approve

). + The gateway slash command parser expects plain text, so HTML tags + prevent command matching. This helper strips tags and decodes entities. + """ + if not text: + return text + # Strip HTML tags + text = re.sub(r"<[^>]+>", "", text) + # Decode common HTML entities + text = text.replace("&", "&").replace("<", "<").replace(">", ">") + text = text.replace(""", '"').replace("'", "'").replace(" ", " ") + return text.strip() + + def _truncate(text: str, limit: int = MAX_ZULIP_MESSAGE) -> str: """Truncate to Zulip's message limit with notice.""" if len(text) <= limit: @@ -302,6 +319,9 @@ class ZulipAdapter(BasePlatformAdapter): # Gen 2: Try to resolve @all-bots user ID dynamically await self._resolve_all_bots_user_id() + # Gen 5: Ensure bot is subscribed to primary stream + await self._ensure_stream_subscriptions() + self._mark_connected() logger.info( "[%s] Connected to %s as %s (queue=%s, bot_id=%s, all_bots_id=%s)", @@ -449,6 +469,10 @@ class ZulipAdapter(BasePlatformAdapter): except Exception: pass + # Clear stale errors on successful poll cycle + self._health_stats["last_error_msg"] = None + self._health_stats["last_error_at"] = None + # Gen 4: Heartbeat logging — prove poll loop is alive now = time.time() if now - self._last_heartbeat_time > HEARTBEAT_INTERVAL: @@ -677,6 +701,64 @@ class ZulipAdapter(BasePlatformAdapter): # Gen 2: Dynamic @all-bots resolution # ------------------------------------------------------------------ + async def _ensure_stream_subscriptions(self) -> None: + """Ensure the bot is subscribed to the primary streams. + + Checks current subscriptions and subscribes to the configured primary + stream (and general-chat) if missing. Without subscription, the bot + receives DMs but no stream events — @mentions in streams are invisible. + + This is a Gen 5 improvement based on lessons from the pi Zulip extension, + where the bot had zero stream subscriptions and couldn't receive + @mentions in stream topics. + """ + try: + # Check current subscriptions + resp, _ = await self._api_call( + "GET", "/api/v1/users/me/subscriptions", + ) + if not resp: + return + + subscribed = resp.get("subscriptions", []) + subscribed_names = [s.get("name", "") for s in subscribed if isinstance(s, dict)] + + # Streams the bot should be subscribed to + required_streams = [self._stream, "general chat"] + missing = [s for s in required_streams if s not in subscribed_names] + + if not missing: + logger.info( + "[%s] Stream subscriptions OK: %s", + self.name, ", ".join(subscribed_names), + ) + return + + # Subscribe to missing streams + import json as _json + payload = {"subscriptions": _json.dumps( + [{"name": s} for s in missing] + )} + sub_resp, _ = await self._api_call( + "POST", "/api/v1/users/me/subscriptions", + data=payload, + ) + if sub_resp and sub_resp.get("result") == "success": + subscribed_str = ", ".join(missing) + logger.info( + "[%s] Subscribed to: %s", self.name, subscribed_str, + ) + self._health_stats["subscriptions_fixed"] = ( + self._health_stats.get("subscriptions_fixed", 0) + 1 + ) + else: + logger.warning( + "[%s] Failed to subscribe to %s", + self.name, ", ".join(missing), + ) + except Exception as e: + logger.warning("[%s] Subscription check failed: %s", self.name, e) + async def _resolve_all_bots_user_id(self) -> None: """Try to resolve the @all-bots user ID from the Zulip server. @@ -719,6 +801,8 @@ class ZulipAdapter(BasePlatformAdapter): sender_name = msg.get("sender_full_name", "Unknown") sender_id = msg.get("sender_id") content = msg.get("content", "") + # Strip Zulip HTML for slash command matching (zulip-approval-fix contract) + content = _strip_html(content) # Echo-loop prevention: skip own messages if sender_email == self._bot_email: @@ -1096,6 +1180,35 @@ class ZulipAdapter(BasePlatformAdapter): "detail": f"@all-bots user_id: {self._all_bots_user_id}", } + # 9. Stream subscriptions (verifies bot is subscribed to at least one stream) + try: + from urllib.parse import urlencode + import json as _json + key_str = f"{self._email}:{self._api_key}" + auth_b64 = base64.b64encode(key_str.encode()).decode() + resp, _ = await self._api_call( + "GET", "/api/v1/users/me/subscriptions", + ) + if resp and "subscriptions" in resp: + stream_count = len(resp["subscriptions"]) + stream_names = [s.get("name", "?") for s in resp["subscriptions"]][:5] + checks["stream_subscriptions"] = { + "status": stream_count > 0, + "detail": f"{stream_count} stream(s): {', '.join(stream_names)}" + if stream_count > 0 + else "No stream subscriptions — bot will not receive stream events", + } + else: + checks["stream_subscriptions"] = { + "status": False, + "detail": "Failed to check subscriptions", + } + except Exception as sub_err: + checks["stream_subscriptions"] = { + "status": False, + "detail": f"Subscription check failed: {sub_err}", + } + # Overall verdict critical = ["connected", "queue_registered", "http_client", "poll_loop"] passed = sum(1 for c in checks.values() if c["status"]) @@ -1311,7 +1424,7 @@ def check_requirements() -> bool: """Check whether the Zulip adapter is installable.""" if not HTTPX_AVAILABLE: return False - site = os.getenv("ZULIP_SITE", "").strip() + site = os.getenv("ZULIP_SITE", "").strip() or os.getenv("ZULIP_URL", "").strip() email = os.getenv("ZULIP_EMAIL", "").strip() api_key = os.getenv("ZULIP_API_KEY", "").strip() return bool(site and email and api_key) @@ -1320,7 +1433,7 @@ def check_requirements() -> bool: def validate_config(config) -> bool: """Validate that the configured Zulip platform has credentials.""" extra = getattr(config, "extra", {}) or {} - site = extra.get("site") or os.getenv("ZULIP_SITE", "") + site = extra.get("site") or os.getenv("ZULIP_SITE", "") or os.getenv("ZULIP_URL", "") email = extra.get("email") or os.getenv("ZULIP_EMAIL", "") api_key = extra.get("api_key") or os.getenv("ZULIP_API_KEY", "") return bool(site and email and api_key) @@ -1333,7 +1446,7 @@ def is_connected(config) -> bool: def _env_enablement() -> Optional[dict]: """Seeds PlatformConfig.extra from env vars for env-only setups.""" - site = os.getenv("ZULIP_SITE", "").strip() + site = os.getenv("ZULIP_SITE", "").strip() or os.getenv("ZULIP_URL", "").strip() email = os.getenv("ZULIP_EMAIL", "").strip() api_key = os.getenv("ZULIP_API_KEY", "").strip() if not (site and email and api_key):