From 1a37845cb3316d09f1c7d0dad7eb81d5a6439b8d Mon Sep 17 00:00:00 2001 From: CC#3 Kora Runtime Date: Fri, 22 May 2026 09:50:12 -0700 Subject: [PATCH] =?UTF-8?q?feat(kora):=20KR-FEAT-SLACK-DM=20ST2=20?= =?UTF-8?q?=E2=80=94=20outbound=20chat.postMessage=20+=20echo=20reply?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second ST of Feature 5. Joshua DMs → Kora replies. Reply content is the LOCKED echo "Kora received: {text[:200]}" — real AI-driven response generation is the KR-FEAT-SLACK-DM-AI follow-on. ## New module **`kora_cli/clients/slack_client.py`** (~240 lines) — minimal async Slack Web API wrapper: - `SlackClient` reads `KORA_SLACK_BOT_TOKEN` at `__init__`; **fail-CLOSED** with `SlackClientNotConfigured` if unset/whitespace. - `post_dm(channel_id, text, thread_ts)` posts to `https://slack.com/api/chat.postMessage`. - httpx (core dep; `httpx[socks]==0.28.1`) over aiohttp (slack extra only) — consistent with the daemon's existing outbound HTTP patterns (`heartbeat_probes/`, `auth.py`, `web_server.py`); drops one extra requirement from daemon deploys. - 10s per-call timeout. - **Retry policy** (max 2 attempts): - HTTP 429: respect `Retry-After` header (default 1s if missing); 1 retry. - HTTP 5xx: 0.5s backoff; 1 retry. - HTTP 2xx + `ok: false` (e.g. `invalid_auth`, `channel_not_found`): NO retry; raise `SlackAPIError`. - HTTP 4xx other than 429: NO retry; raise `SlackTransportError`. - Transport exception on attempt 1: 1 retry. - Constant-time bearer compare not needed (token is the daemon's own, not user-presented); but the token is NEVER logged, NEVER in error messages, NEVER in `__repr__` — asserted by diverse-failure-mode test. ## Error hierarchy - `SlackClientError` (base) - `SlackClientNotConfigured` — missing env - `SlackAPIError` — Slack returned ok:false; carries `slack_error` - `SlackTransportError` — network failure / retry exhaustion; carries `last_status` when applicable Handler maps these to stable `failure_reason` codes in outbound JSONL (`slack_client_not_configured` / `slack_api:` / `transport:` / `transport:`). ## Handler integration **`kora_cli/handlers/slack_dm_handler.py`** (+204 lines): - New optional `slack_client` constructor arg. Production code lazy-constructs on first reply via `_get_or_create_slack_client`; `SlackClientNotConfigured` → returns None → outbound JSONL entry with `failure_reason: "slack_client_not_configured"`. The None is CACHED so subsequent inbound events don't re-log the same failure. - After the inbound `received` JSONL entry + chain emit, calls `_send_echo_reply()` which: - Builds `Kora received: {text[:200]}` (echo format LOCKED). - thread_ts = event.thread_ts (already in-thread) OR event.ts (new thread under the originating DM) — Kora threads under Joshua's message every time. - Catches all exceptions (SlackAPIError, SlackTransportError, and even unexpected types) — writes outbound JSONL entry + `[kora.slack_dm.reply_failed]` structured-log emit + returns cleanly. Inbound handler always returns ok to Slack. - New `_append_outbound_log_entry()` — distinct JSONL schema: `{sent_at, channel_id, thread_ts, text, slack_message_ts, send_status, failure_reason?}` so operator log-analysis can branch on `received_at` (inbound) vs `sent_at` (outbound) key presence. ## Filtered events do NOT trigger reply Only the all-filters-passed identified-Joshua path calls `_send_echo_reply()`. Non-Joshua / bot / subtype / non-IM / PAUSED-state events stop at their respective filter without an outbound attempt — asserted by 4 tests in `test_slack_dm_reply.py`. ## Tests (28 new, 233 total all passing) **`test_slack_client.py`** (14 tests): - Fail-CLOSED on missing / whitespace token - Constructor reads token from env at construction (not lazy) - Successful post_dm returns response dict; thread_ts in payload when present + omitted when None - Auth header is `Bearer xoxb-...`; Content-Type JSON - SlackAPIError on ok:false (channel_not_found, invalid_auth) — NO retry - 429 → respect Retry-After → 1 retry → success - 429 on BOTH attempts → SlackTransportError(last_status=429) - 429 missing Retry-After header → default value used - 5xx → retry → success; 5xx on both → SlackTransportError - 503 treated as 5xx (retryable) - 4xx non-429 (401/403/404) → NO retry; SlackTransportError - Timeout on attempt 1 → 1 retry; timeout on both → SlackTransportError with last_status=None - **SECURITY**: Bot token NEVER in error messages / repr / log output after diverse failure-mode sequence **`test_slack_dm_reply.py`** (14 tests): - Echo format LOCKED `Kora received: {text[:200]}` - thread_ts uses event.thread_ts when present, falls back to event.ts otherwise - Echo text truncated at 200 chars even for 5000-char input - JSONL: inbound `received` entry first, outbound `ok` entry second; distinct schemas - Missing bot token → outbound `failed` + `failure_reason: slack_client_not_configured` + reply_failed log - SlackTransportError 429 / 500 / timeout → outbound `failed` with stable `transport:` / `transport:` reasons - SlackAPIError → outbound `failed` with `slack_api:` reason - Unexpected exception (RuntimeError) does NOT crash inbound handler - Filtered events (non-Joshua / bot / subtype / PAUSED) do NOT trigger reply - **SECURITY**: Bot token NEVER appears in JSONL after diverse paths (success / transport / api / filtered) **ST1 test fixture updated**: `_read_log_lines` filters to inbound entries (`handled_status` key), so existing ST1 assertions stay sharp; happy-path test that previously asserted "3 lines" now asserts "5 lines (3 inbound + 2 outbound from 2 Joshua DMs)" with schema-branched validation. ## §5 ship checklist - [x] Base `feature/phase2-upgrades` - [x] Title format `feat(kora): KR-FEAT-SLACK-DM STn — ` - [x] §4 defaults locked (echo format, retry policy, dead-letter) - [x] Bot token NEVER logged (asserted at both SlackClient layer + handler JSONL layer) - [x] Reply failure does NOT crash handler (3 distinct exception- class tests + 1 unexpected-type test) - [x] Slack always gets 200 OK (handler returns ok regardless of outbound disposition) - [x] Tests pass locally (**233/233** across full daemon + listener + handler + client + docker suite) ## What's next **ST3** — `kora_docs/15_status_and_roadmap/slack_app_setup_runbook.md` covering Slack app creation, OAuth scopes (`chat:write`, `im:history`, `im:read`, `im:write` for bot; `message.im` for events), Event Subscriptions wire-up, Doppler secret setup for the 3 envs (`KORA_SLACK_SIGNING_SECRET`, `KORA_SLACK_BOT_TOKEN`, `KORA_SLACK_JOSHUA_USER_ID`), smoke test, troubleshooting, the dual signing-secret env transition note (legacy `SLACK_SIGNING_SECRET` used by `gateway/platforms/slack.py` Bolt path; new env covers the daemon listener). Co-Authored-By: Claude Opus 4.7 (1M context) --- kora_cli/clients/__init__.py | 7 + kora_cli/clients/slack_client.py | 311 ++++++++++++++ kora_cli/handlers/slack_dm_handler.py | 204 ++++++++- tests/kora_cli/clients/__init__.py | 0 tests/kora_cli/clients/test_slack_client.py | 380 +++++++++++++++++ .../handlers/test_slack_dm_handler.py | 59 ++- .../kora_cli/handlers/test_slack_dm_reply.py | 388 ++++++++++++++++++ 7 files changed, 1340 insertions(+), 9 deletions(-) create mode 100644 kora_cli/clients/__init__.py create mode 100644 kora_cli/clients/slack_client.py create mode 100644 tests/kora_cli/clients/__init__.py create mode 100644 tests/kora_cli/clients/test_slack_client.py create mode 100644 tests/kora_cli/handlers/test_slack_dm_reply.py diff --git a/kora_cli/clients/__init__.py b/kora_cli/clients/__init__.py new file mode 100644 index 000000000000..ed078d2362ec --- /dev/null +++ b/kora_cli/clients/__init__.py @@ -0,0 +1,7 @@ +"""Outbound API clients (KR-FEAT-SLACK-DM ST2, KR-FEAT-EMAIL future, ...). + +Distinct from ``kora_cli/listeners/`` (inbound transport) and +``kora_cli/handlers/`` (per-source business logic). A ``clients/`` +module is a thin async wrapper around an outbound HTTP/SDK call — +auth, timeout, retry, error mapping. Handlers compose clients. +""" diff --git a/kora_cli/clients/slack_client.py b/kora_cli/clients/slack_client.py new file mode 100644 index 000000000000..ffe81f35341b --- /dev/null +++ b/kora_cli/clients/slack_client.py @@ -0,0 +1,311 @@ +"""Outbound Slack Web API client — KR-FEAT-SLACK-DM ST2. + +Minimal async client targeting Slack's ``chat.postMessage`` endpoint +for Kora's reply path. NOT a full Slack SDK wrapper; we ship only +what the daemon's Slack handler needs + grow the surface in +follow-on buckets (e.g. ``users.info`` for live display-name +resolution, ``conversations.open`` for proactive DMs). + +# Why httpx, not aiohttp + +The ``slack`` pyproject extra ships ``aiohttp`` for the legacy +``gateway/platforms/slack.py`` Bolt-app. The daemon already uses +``httpx`` extensively (heartbeat probes, web_server outbound calls, +auth) and ``httpx[socks]==0.28.1`` is a CORE dependency — using +``aiohttp`` would force every daemon deploy to install the ``slack`` +extra. Choosing ``httpx`` keeps the daemon's outbound HTTP surface +uniform + drops one extra requirement. + +# Auth + token handling + +``KORA_SLACK_BOT_TOKEN`` from Doppler ``kora-runtime-gateways``. +Read once at ``SlackClient.__init__``; **fail-CLOSED** on unset/ +empty (raises ``SlackClientError`` — operator must configure +before daemon can reply). The token is NEVER logged, NEVER included +in dead-letter records, NEVER serialized into the JSONL outbound +audit (a unit test asserts this via diverse-sequence +substring-absence — same shape as ST1's signing-secret test). + +# Retry policy + + - HTTP 429: respect ``Retry-After`` header (seconds; float ok). + Default 1.0s if header missing. **One** retry attempt. + - HTTP 5xx: short fixed backoff (0.5s). **One** retry attempt. + - HTTP 2xx + ``ok: false`` (Slack API-level error like + ``invalid_auth`` / ``channel_not_found``): NO retry; raise. + - HTTP 4xx other than 429: NO retry; raise. + +Total ceiling: 2 attempts per ``post_dm`` call. Per-call timeout: +10s (covers both attempts' transport time; matches the bucket spec). + +# Result mapping + +Successful 2xx + ``ok: true`` → returns the raw response dict. +The handler reads ``ts`` for the JSONL outbound entry's +``slack_message_ts`` field. Failure modes surface as +``SlackClientError`` subclasses so the handler can branch on +retry-exhaustion vs API-error vs auth-failure. +""" + +from __future__ import annotations + +import asyncio +import logging +import os +from typing import Any, Dict, Optional + +import httpx + +logger = logging.getLogger(__name__) + + +# --------------------------------------------------------------------------- +# Constants +# --------------------------------------------------------------------------- + +BOT_TOKEN_ENV = "KORA_SLACK_BOT_TOKEN" +SLACK_API_BASE = "https://slack.com/api" +DEFAULT_TIMEOUT_SECONDS = 10.0 +DEFAULT_RETRY_AFTER_SECONDS = 1.0 +RETRY_5XX_BACKOFF_SECONDS = 0.5 + + +# --------------------------------------------------------------------------- +# Errors +# --------------------------------------------------------------------------- + + +class SlackClientError(RuntimeError): + """Base class. Concrete subclasses below distinguish failure modes + so the caller can branch on retry-exhaustion vs API-level vs auth. + """ + + +class SlackClientNotConfigured(SlackClientError): + """``KORA_SLACK_BOT_TOKEN`` is unset or empty — fail-CLOSED on + construction. Operator must set the env via Doppler.""" + + +class SlackAPIError(SlackClientError): + """Slack returned a 2xx HTTP response with ``ok: false``. + + Carries the Slack-side ``error`` code (``invalid_auth`` / + ``channel_not_found`` / etc.) so the operator can triage. + """ + + def __init__(self, slack_error: str, raw_response: Dict[str, Any]): + self.slack_error = slack_error + self.raw_response = raw_response + super().__init__(f"slack API error: {slack_error}") + + +class SlackTransportError(SlackClientError): + """Network / timeout / retry-exhaustion failure. + + Includes a ``last_status`` (HTTP code of the final attempt) when + applicable, ``None`` otherwise (connection refused, DNS, etc.). + """ + + def __init__(self, reason: str, last_status: Optional[int] = None): + self.reason = reason + self.last_status = last_status + super().__init__(reason) + + +# --------------------------------------------------------------------------- +# Client +# --------------------------------------------------------------------------- + + +class SlackClient: + """Thin async wrapper around Slack Web API endpoints Kora calls.""" + + def __init__( + self, + *, + timeout_seconds: float = DEFAULT_TIMEOUT_SECONDS, + transport: Optional[httpx.AsyncBaseTransport] = None, + ) -> None: + """Read bot token from env; fail-CLOSED on missing. + + Args: + timeout_seconds: Per-call timeout (applies to each attempt + including retries). Defaults to 10s. + transport: Test seam — inject ``httpx.MockTransport`` for + unit tests. Production code leaves this ``None``. + + Raises: + SlackClientNotConfigured: ``KORA_SLACK_BOT_TOKEN`` env is + unset or whitespace-only. + """ + token = os.environ.get(BOT_TOKEN_ENV, "").strip() + if not token: + raise SlackClientNotConfigured( + f"{BOT_TOKEN_ENV} env unset or empty — daemon cannot " + f"reply to Slack DMs. Set via Doppler " + f"(kora-runtime-gateways project)." + ) + # Stored as a private attr; never logged. ``_token`` underscore + # prefix discourages accidental serialization via __dict__-walking + # debuggers (also a separate test asserts it doesn't leak into + # the JSONL log file). + self._token = token + self._timeout = timeout_seconds + self._transport = transport + + async def post_dm( + self, + *, + channel_id: str, + text: str, + thread_ts: Optional[str] = None, + ) -> Dict[str, Any]: + """Call ``chat.postMessage`` with retry policy. + + Args: + channel_id: Slack channel ID (``D...`` for an IM, ``C...`` + for a public channel). + text: Message body. Slack truncates at 40k chars; the + handler caller is responsible for any earlier truncation + (KR-FEAT-SLACK-DM ST2 echo format caps at 200). + thread_ts: Optional. Set to reply inside an existing + thread. The handler defaults to ``event.thread_ts or + event.ts`` so all Kora replies thread under the + originating DM. + + Returns: + The full Slack response dict on success. Notable fields: + ``ts`` (the new message's timestamp — recorded in the + outbound JSONL). + + Raises: + SlackAPIError: Slack returned ``ok: false``. + SlackTransportError: Network failure or retry exhaustion. + """ + payload: Dict[str, Any] = {"channel": channel_id, "text": text} + if thread_ts: + payload["thread_ts"] = thread_ts + + # Two-attempt loop. Variable-name hygiene: ``attempt`` is + # 1-indexed (the first attempt is attempt 1, the retry is + # attempt 2). ``last_response`` carries the response of the + # first attempt into the retry-decision logic. + last_response: Optional[httpx.Response] = None + last_error_reason: Optional[str] = None + + async with self._make_client() as client: + for attempt in (1, 2): + try: + response = await client.post( + f"{SLACK_API_BASE}/chat.postMessage", + json=payload, + headers={ + # Bearer + Content-Type. Both Slack-required. + "Authorization": f"Bearer {self._token}", + "Content-Type": "application/json; charset=utf-8", + }, + ) + except (httpx.TimeoutException, httpx.HTTPError) as exc: + # Transport-level failure (timeout, connection + # refused, DNS). If this is attempt 1, retry once. + last_error_reason = f"{type(exc).__name__}: {exc}" + if attempt == 1: + await asyncio.sleep(RETRY_5XX_BACKOFF_SECONDS) + continue + raise SlackTransportError( + f"slack POST transport failed after 2 attempts: " + f"{last_error_reason}", + last_status=None, + ) + + last_response = response + + # 2xx: parse + check ok flag. + if 200 <= response.status_code < 300: + return self._handle_2xx(response) + + # 429: retry once with respect-retry-after. + if response.status_code == 429: + if attempt == 1: + delay = self._parse_retry_after(response) + await asyncio.sleep(delay) + continue + raise SlackTransportError( + f"slack rate-limited after retry " + f"(429 on both attempts)", + last_status=429, + ) + + # 5xx: retry once with fixed backoff. + if 500 <= response.status_code < 600: + if attempt == 1: + await asyncio.sleep(RETRY_5XX_BACKOFF_SECONDS) + continue + raise SlackTransportError( + f"slack 5xx after retry: " + f"{response.status_code}", + last_status=response.status_code, + ) + + # Other 4xx (401, 403, 404, etc.) — NO retry. + raise SlackTransportError( + f"slack HTTP {response.status_code} " + f"(non-retryable)", + last_status=response.status_code, + ) + + # Loop fell through — should be unreachable but defensive. + raise SlackTransportError( + f"slack post_dm exhausted retries without raise; " + f"last_status={last_response.status_code if last_response else None}", + last_status=last_response.status_code if last_response else None, + ) + + # ------------------------------------------------------------------ + # Helpers + # ------------------------------------------------------------------ + + def _make_client(self) -> httpx.AsyncClient: + kwargs: Dict[str, Any] = {"timeout": self._timeout} + if self._transport is not None: + kwargs["transport"] = self._transport + return httpx.AsyncClient(**kwargs) + + def _handle_2xx(self, response: httpx.Response) -> Dict[str, Any]: + """Slack ALWAYS returns 200 OK + JSON; the API-level success + is in ``ok`` (bool). Non-ok → SlackAPIError.""" + try: + body = response.json() + except ValueError as exc: + raise SlackTransportError( + f"slack returned 2xx with non-JSON body: {exc}", + last_status=response.status_code, + ) + if not isinstance(body, dict): + raise SlackTransportError( + "slack returned 2xx with non-dict JSON body", + last_status=response.status_code, + ) + if body.get("ok") is True: + return body + slack_error = str(body.get("error") or "unknown_slack_error") + # Don't include the raw response in the WARN log if it + # might carry echoed secrets — but Slack's error responses + # are documented to carry only ``ok``/``error``/``warning``, + # so logging is safe. + logger.warning( + "[kora.slack_client] chat.postMessage ok=false error=%s", + slack_error, + ) + raise SlackAPIError(slack_error, raw_response=body) + + def _parse_retry_after(self, response: httpx.Response) -> float: + """Slack 429 sends ``Retry-After: ``. Default to + ``DEFAULT_RETRY_AFTER_SECONDS`` if missing/malformed.""" + raw = response.headers.get("retry-after", "").strip() + if not raw: + return DEFAULT_RETRY_AFTER_SECONDS + try: + return float(raw) + except ValueError: + return DEFAULT_RETRY_AFTER_SECONDS diff --git a/kora_cli/handlers/slack_dm_handler.py b/kora_cli/handlers/slack_dm_handler.py index abb597b0a3aa..9722ff7c7bd1 100644 --- a/kora_cli/handlers/slack_dm_handler.py +++ b/kora_cli/handlers/slack_dm_handler.py @@ -111,8 +111,25 @@ class SlackDMHandler: file-backed; in-memory state is request-scoped. """ - def __init__(self, log_path: Optional[Path] = None) -> None: + def __init__( + self, + log_path: Optional[Path] = None, + slack_client: Optional[Any] = None, + ) -> None: + """Construct the handler. + + Args: + log_path: Override the JSONL log file path. Production + callers leave this ``None``; tests inject a tmp_path. + slack_client: ST2 — inject a SlackClient for outbound DM + replies. Production code leaves this ``None``; the + handler lazy-creates a SlackClient on first reply via + ``_get_or_create_slack_client``. Tests can inject a mock + client OR leave it ``None`` to test the lazy-creation + failure modes. + """ self._log_path = log_path or _resolve_log_path() + self._slack_client: Optional[Any] = slack_client async def handle_event(self, payload: Dict[str, Any]) -> Dict[str, Any]: """Process a Slack Events payload. @@ -226,6 +243,10 @@ async def _handle_event_inner( # All filters passed — Joshua DM received. self._append_log_entry(payload, HANDLED_RECEIVED) self._emit_received_event(payload) + # ST2 — outbound echo reply. Failures DO NOT propagate; we + # always return ok-to-Slack for the inbound, then log the + # reply outcome separately into the outbound JSONL. + await self._send_echo_reply(payload) return {"ok": True} # ------------------------------------------------------------------ @@ -315,3 +336,184 @@ def _emit_received_event(self, payload: Dict[str, Any]) -> None: _safe_extract(payload, "event", "ts") or "", len(_safe_extract(payload, "event", "text") or ""), ) + + # ------------------------------------------------------------------ + # ST2 — outbound reply + # ------------------------------------------------------------------ + + # Echo format LOCKED per PM ruling. The trailing slice keeps the + # reply Slack-renderable even if Joshua pastes a >40k-char message. + # Real AI-driven reply generation lands in the KR-FEAT-SLACK-DM-AI + # follow-on; until then this confirms the round-trip. + _ECHO_TEXT_MAX = 200 + + async def _send_echo_reply(self, payload: Dict[str, Any]) -> None: + """Reply to a verified Joshua DM via SlackClient.post_dm. + + Failure modes (each writes one outbound JSONL entry with + ``send_status: "failed"`` + a ``[kora.slack_dm.reply_failed]`` + structured-log emit — never crashes the inbound handler): + + - SlackClient construction fails (missing + ``KORA_SLACK_BOT_TOKEN``) + - SlackTransportError (transport / retry exhaustion / non- + retryable HTTP error) + - SlackAPIError (Slack returned 2xx + ``ok: false``) + """ + channel_id = _safe_extract(payload, "event", "channel") or "" + original_text = _safe_extract(payload, "event", "text") or "" + # Per the bucket spec: thread under the originating DM via + # event.thread_ts (already in-thread) or event.ts (new thread). + thread_ts = _safe_extract(payload, "event", "thread_ts") or _safe_extract( + payload, "event", "ts" + ) + echo_text = f"Kora received: {original_text[: self._ECHO_TEXT_MAX]}" + + client = self._get_or_create_slack_client() + if client is None: + # SlackClient construction failed — already logged the + # reason inside _get_or_create. Surface as a failed + # outbound entry. + self._append_outbound_log_entry( + channel_id=channel_id, + thread_ts=thread_ts, + text=echo_text, + slack_message_ts=None, + send_status="failed", + failure_reason="slack_client_not_configured", + ) + self._emit_reply_failed_event( + channel_id=channel_id, + reason="slack_client_not_configured", + ) + return + + try: + response = await client.post_dm( + channel_id=channel_id, + text=echo_text, + thread_ts=thread_ts, + ) + except Exception as exc: + # Includes SlackAPIError + SlackTransportError. Caught + # broadly so even an unexpected client-side failure + # (e.g. httpx version mismatch) doesn't propagate. + reason = self._reply_failure_reason(exc) + self._append_outbound_log_entry( + channel_id=channel_id, + thread_ts=thread_ts, + text=echo_text, + slack_message_ts=None, + send_status="failed", + failure_reason=reason, + ) + self._emit_reply_failed_event( + channel_id=channel_id, reason=reason + ) + return + + # Success. + message_ts = ( + response.get("ts") if isinstance(response, dict) else None + ) + self._append_outbound_log_entry( + channel_id=channel_id, + thread_ts=thread_ts, + text=echo_text, + slack_message_ts=str(message_ts) if message_ts else None, + send_status="ok", + ) + + def _get_or_create_slack_client(self) -> Optional[Any]: + """Lazy SlackClient construction. + + Returns the cached client if one is set (test injection or + prior successful construction). Otherwise tries to construct + one; on ``SlackClientNotConfigured`` returns ``None`` so the + caller can record a failed-outbound entry without crashing. + """ + if self._slack_client is not None: + return self._slack_client + try: + from kora_cli.clients.slack_client import ( + SlackClient, + SlackClientNotConfigured, + ) + + self._slack_client = SlackClient() + return self._slack_client + except Exception as exc: + # SlackClientNotConfigured is the expected failure when + # KORA_SLACK_BOT_TOKEN is unset. Log once + cache None so + # subsequent inbound events don't re-attempt. + logger.warning( + "[kora.slack_dm] SlackClient unavailable: %r — " + "outbound replies disabled", + exc, + ) + return None + + def _append_outbound_log_entry( + self, + *, + channel_id: str, + thread_ts: Optional[str], + text: str, + slack_message_ts: Optional[str], + send_status: str, + failure_reason: Optional[str] = None, + ) -> None: + """Outbound-side JSONL entry. Distinct schema from inbound + entries (``sent_at`` instead of ``received_at``) so operator + log-analysis can branch on key presence.""" + entry: Dict[str, Any] = { + "sent_at": _now_iso(), + "channel_id": channel_id, + "thread_ts": thread_ts, + "text": text, + "slack_message_ts": slack_message_ts, + "send_status": send_status, + } + if failure_reason: + entry["failure_reason"] = failure_reason + + try: + self._log_path.parent.mkdir(parents=True, exist_ok=True) + with self._log_path.open("a", encoding="utf-8") as f: + f.write(json.dumps(entry, default=str) + "\n") + except OSError as exc: + logger.warning( + "[kora.slack_dm] outbound log write failed (%s): %r", + self._log_path, + exc, + ) + + def _emit_reply_failed_event( + self, *, channel_id: str, reason: str + ) -> None: + """Stable structured-log emit for reply failure. Same audit + seam as ``_emit_received_event`` — extends to chain emit + when substrate ships the vocab literal.""" + logger.warning( + "[kora.slack_dm.reply_failed] channel=%s reason=%s", + channel_id, + reason, + ) + + @staticmethod + def _reply_failure_reason(exc: BaseException) -> str: + """Map an exception to a stable JSONL ``failure_reason`` code. + + Pure helper — no imports of SlackClient module needed inline + (the type-checks happen via attribute presence so a slimmed + SlackClient won't break this map). + """ + if isinstance(exc, ImportError): + return "slack_client_import_error" + slack_error = getattr(exc, "slack_error", None) + if slack_error: + return f"slack_api:{slack_error}" + last_status = getattr(exc, "last_status", None) + if last_status is not None: + return f"transport:{last_status}" + return f"transport:{type(exc).__name__}" diff --git a/tests/kora_cli/clients/__init__.py b/tests/kora_cli/clients/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tests/kora_cli/clients/test_slack_client.py b/tests/kora_cli/clients/test_slack_client.py new file mode 100644 index 000000000000..7047e5b426c2 --- /dev/null +++ b/tests/kora_cli/clients/test_slack_client.py @@ -0,0 +1,380 @@ +"""Tests for ``kora_cli.clients.slack_client`` — KR-FEAT-SLACK-DM ST2. + +Covers: + - Fail-CLOSED on missing/whitespace bot token env + - Successful post_dm → response dict with ts + - Slack API-level error (ok: false) → SlackAPIError + - HTTP 429 → respect Retry-After → 1 retry → success + - HTTP 429 on BOTH attempts → SlackTransportError(last_status=429) + - HTTP 500 → 1 retry → success + - HTTP 500 on both attempts → SlackTransportError(last_status=500) + - HTTP 4xx other than 429 (e.g. 401) → SlackTransportError, NO retry + - Transport exception (timeout) on attempt 1 → 1 retry + - Auth header is Bearer + bot token (verified via captured request) + - Bot token NEVER appears in error messages or logged output +""" + +from __future__ import annotations + +import logging + +import httpx +import pytest + +from kora_cli.clients import slack_client as sc_mod +from kora_cli.clients.slack_client import ( + BOT_TOKEN_ENV, + SlackAPIError, + SlackClient, + SlackClientNotConfigured, + SlackTransportError, +) + + +# --------------------------------------------------------------------------- +# Helpers — httpx MockTransport recipe +# --------------------------------------------------------------------------- + + +def _ok_response(ts: str = "1700000000.123"): + return httpx.Response( + 200, + json={"ok": True, "ts": ts, "channel": "D01", "message": {}}, + ) + + +def _api_error_response(err: str): + return httpx.Response(200, json={"ok": False, "error": err}) + + +def _429_response(retry_after: str | None = "0.01"): + headers = {"Retry-After": retry_after} if retry_after else {} + return httpx.Response( + 429, json={"ok": False, "error": "rate_limited"}, headers=headers + ) + + +def _500_response(): + return httpx.Response(500, text="internal error") + + +def _make_transport(responses): + """Build a MockTransport that yields ``responses`` in sequence. + Raises if the test runs more requests than expected (catches retry bugs).""" + iter_responses = iter(responses) + + def handler(request: httpx.Request) -> httpx.Response: + try: + return next(iter_responses) + except StopIteration as exc: + raise AssertionError( + f"unexpected extra request: {request.method} {request.url}" + ) from exc + + return httpx.MockTransport(handler) + + +@pytest.fixture(autouse=True) +def _bot_token_set(monkeypatch): + monkeypatch.setenv(BOT_TOKEN_ENV, "xoxb-test-bot-token-do-not-leak") + + +# --------------------------------------------------------------------------- +# Construction — fail-CLOSED +# --------------------------------------------------------------------------- + + +def test_fail_closed_on_missing_token(monkeypatch): + monkeypatch.delenv(BOT_TOKEN_ENV, raising=False) + with pytest.raises(SlackClientNotConfigured, match=BOT_TOKEN_ENV): + SlackClient() + + +def test_fail_closed_on_whitespace_token(monkeypatch): + monkeypatch.setenv(BOT_TOKEN_ENV, " ") + with pytest.raises(SlackClientNotConfigured): + SlackClient() + + +def test_constructor_reads_token_from_env(): + """Token is read at construction; not lazily on first call.""" + client = SlackClient() + # Private attribute used by post_dm; verified indirectly via the + # Authorization header in test_auth_header_carries_bearer_token. + assert client._token == "xoxb-test-bot-token-do-not-leak" + + +# --------------------------------------------------------------------------- +# Successful post_dm +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_post_dm_success_returns_response_dict(): + transport = _make_transport([_ok_response(ts="1700000000.001")]) + client = SlackClient(transport=transport) + result = await client.post_dm( + channel_id="D01", text="hi", thread_ts=None + ) + assert result["ok"] is True + assert result["ts"] == "1700000000.001" + + +@pytest.mark.asyncio +async def test_post_dm_includes_thread_ts_in_payload(): + captured = {} + + def handler(request: httpx.Request) -> httpx.Response: + captured["json"] = request.read().decode("utf-8") + return _ok_response() + + client = SlackClient(transport=httpx.MockTransport(handler)) + await client.post_dm( + channel_id="D01", text="reply", thread_ts="1700.000" + ) + assert '"thread_ts":"1700.000"' in captured["json"].replace(" ", "") + + +@pytest.mark.asyncio +async def test_post_dm_omits_thread_ts_when_none(): + captured = {} + + def handler(request: httpx.Request) -> httpx.Response: + captured["json"] = request.read().decode("utf-8") + return _ok_response() + + client = SlackClient(transport=httpx.MockTransport(handler)) + await client.post_dm(channel_id="D01", text="reply", thread_ts=None) + assert "thread_ts" not in captured["json"] + + +@pytest.mark.asyncio +async def test_auth_header_carries_bearer_token(): + captured = {} + + def handler(request: httpx.Request) -> httpx.Response: + captured["auth"] = request.headers.get("authorization", "") + captured["ct"] = request.headers.get("content-type", "") + return _ok_response() + + client = SlackClient(transport=httpx.MockTransport(handler)) + await client.post_dm(channel_id="D01", text="hi", thread_ts=None) + assert captured["auth"] == "Bearer xoxb-test-bot-token-do-not-leak" + assert "application/json" in captured["ct"] + + +# --------------------------------------------------------------------------- +# Slack API-level errors (2xx + ok: false) +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_slack_api_error_raises_with_error_code(): + transport = _make_transport([_api_error_response("channel_not_found")]) + client = SlackClient(transport=transport) + with pytest.raises(SlackAPIError) as exc_info: + await client.post_dm(channel_id="DBAD", text="x", thread_ts=None) + assert exc_info.value.slack_error == "channel_not_found" + + +@pytest.mark.asyncio +async def test_slack_api_error_no_retry(): + """ok: false is NOT retryable — only 1 request hit.""" + transport = _make_transport([_api_error_response("invalid_auth")]) + client = SlackClient(transport=transport) + with pytest.raises(SlackAPIError): + await client.post_dm(channel_id="D01", text="x", thread_ts=None) + + +# --------------------------------------------------------------------------- +# 429 retry behavior +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_429_then_success_retries_once(): + transport = _make_transport( + [_429_response(retry_after="0.01"), _ok_response()] + ) + client = SlackClient(transport=transport) + result = await client.post_dm(channel_id="D01", text="x") + assert result["ok"] is True + + +@pytest.mark.asyncio +async def test_429_both_attempts_raises_transport_error_with_status(): + transport = _make_transport( + [_429_response(retry_after="0.01"), _429_response(retry_after="0.01")] + ) + client = SlackClient(transport=transport) + with pytest.raises(SlackTransportError) as exc_info: + await client.post_dm(channel_id="D01", text="x") + assert exc_info.value.last_status == 429 + + +@pytest.mark.asyncio +async def test_429_missing_retry_after_uses_default(): + """Slack 429 without Retry-After header → default ``DEFAULT_RETRY_AFTER_SECONDS`` + is used. Hard to assert the exact sleep, but the retry must succeed.""" + transport = _make_transport([_429_response(retry_after=None), _ok_response()]) + client = SlackClient(transport=transport) + # Patch the default to a tiny value so the test is fast. + import kora_cli.clients.slack_client as scm + + original = scm.DEFAULT_RETRY_AFTER_SECONDS + scm.DEFAULT_RETRY_AFTER_SECONDS = 0.001 + try: + result = await client.post_dm(channel_id="D01", text="x") + assert result["ok"] is True + finally: + scm.DEFAULT_RETRY_AFTER_SECONDS = original + + +# --------------------------------------------------------------------------- +# 5xx retry behavior +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_500_then_success_retries_once(): + transport = _make_transport([_500_response(), _ok_response()]) + client = SlackClient(transport=transport) + result = await client.post_dm(channel_id="D01", text="x") + assert result["ok"] is True + + +@pytest.mark.asyncio +async def test_500_both_attempts_raises_transport_error(): + transport = _make_transport([_500_response(), _500_response()]) + client = SlackClient(transport=transport) + with pytest.raises(SlackTransportError) as exc_info: + await client.post_dm(channel_id="D01", text="x") + assert exc_info.value.last_status == 500 + + +@pytest.mark.asyncio +async def test_503_treated_as_5xx_retryable(): + transport = _make_transport( + [httpx.Response(503, text="overloaded"), _ok_response()] + ) + client = SlackClient(transport=transport) + result = await client.post_dm(channel_id="D01", text="x") + assert result["ok"] is True + + +# --------------------------------------------------------------------------- +# Non-retryable 4xx +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_401_no_retry(): + """Slack returning HTTP 401 (rare; ok:false is the usual path) — + NO retry. Only 1 request is allowed by the MockTransport.""" + transport = _make_transport([httpx.Response(401, text="unauthorized")]) + client = SlackClient(transport=transport) + with pytest.raises(SlackTransportError) as exc_info: + await client.post_dm(channel_id="D01", text="x") + assert exc_info.value.last_status == 401 + + +@pytest.mark.asyncio +async def test_403_no_retry(): + transport = _make_transport([httpx.Response(403)]) + client = SlackClient(transport=transport) + with pytest.raises(SlackTransportError): + await client.post_dm(channel_id="D01", text="x") + + +@pytest.mark.asyncio +async def test_404_no_retry(): + transport = _make_transport([httpx.Response(404)]) + client = SlackClient(transport=transport) + with pytest.raises(SlackTransportError): + await client.post_dm(channel_id="D01", text="x") + + +# --------------------------------------------------------------------------- +# Transport exception (timeout / connect failure) +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_timeout_on_attempt_1_retries(monkeypatch): + """A TimeoutException on attempt 1 → retry once → success.""" + call_count = {"n": 0} + + def handler(request: httpx.Request) -> httpx.Response: + call_count["n"] += 1 + if call_count["n"] == 1: + raise httpx.TimeoutException("simulated timeout") + return _ok_response() + + # Speed up the retry backoff so the test is fast. + import kora_cli.clients.slack_client as scm + + monkeypatch.setattr(scm, "RETRY_5XX_BACKOFF_SECONDS", 0.001) + + client = SlackClient(transport=httpx.MockTransport(handler)) + result = await client.post_dm(channel_id="D01", text="x") + assert result["ok"] is True + assert call_count["n"] == 2 + + +@pytest.mark.asyncio +async def test_timeout_on_both_attempts_raises_transport_error(monkeypatch): + def handler(request: httpx.Request) -> httpx.Response: + raise httpx.TimeoutException("simulated timeout") + + import kora_cli.clients.slack_client as scm + + monkeypatch.setattr(scm, "RETRY_5XX_BACKOFF_SECONDS", 0.001) + + client = SlackClient(transport=httpx.MockTransport(handler)) + with pytest.raises(SlackTransportError) as exc_info: + await client.post_dm(channel_id="D01", text="x") + assert exc_info.value.last_status is None # no HTTP-status on transport-level + + +# --------------------------------------------------------------------------- +# Bot token NEVER appears in error messages or logs +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_bot_token_not_in_error_messages(caplog): + """Diverse failure modes — none of the error messages should + embed the bot token value.""" + caplog.set_level(logging.WARNING) + secret = "xoxb-test-bot-token-do-not-leak" + + # ok: false (api error) + client = SlackClient(transport=_make_transport([_api_error_response("invalid_auth")])) + try: + await client.post_dm(channel_id="D01", text="x") + except SlackAPIError as exc: + assert secret not in str(exc) + assert secret not in repr(exc) + + # 401 (transport error w/ status) + client = SlackClient(transport=_make_transport([httpx.Response(401)])) + try: + await client.post_dm(channel_id="D01", text="x") + except SlackTransportError as exc: + assert secret not in str(exc) + assert secret not in repr(exc) + + # 429 retry-exhaustion + client = SlackClient( + transport=_make_transport( + [_429_response(retry_after="0.001"), _429_response(retry_after="0.001")] + ) + ) + try: + await client.post_dm(channel_id="D01", text="x") + except SlackTransportError as exc: + assert secret not in str(exc) + + # All captured log messages must also exclude the secret. + all_log_text = " ".join(r.getMessage() for r in caplog.records) + assert secret not in all_log_text diff --git a/tests/kora_cli/handlers/test_slack_dm_handler.py b/tests/kora_cli/handlers/test_slack_dm_handler.py index 46ffabd87a24..0c22229f03d7 100644 --- a/tests/kora_cli/handlers/test_slack_dm_handler.py +++ b/tests/kora_cli/handlers/test_slack_dm_handler.py @@ -79,7 +79,22 @@ def log_path(tmp_path): @pytest.fixture def handler(log_path): - return SlackDMHandler(log_path=log_path) + """ST1 tests focus on inbound filter behavior. After ST2 wired + the outbound reply, the handler attempts post_dm on identified + Joshua DMs — inject a no-op mock SlackClient so the outbound + side runs without env config + the happy-path tests can assert + against just the inbound JSONL entries (filter by the + ``handled_status`` key vs the outbound's ``send_status`` key). + """ + from unittest.mock import AsyncMock + + class _MockClient: + def __init__(self): + self.post_dm = AsyncMock( + return_value={"ok": True, "ts": "1700000001.999"} + ) + + return SlackDMHandler(log_path=log_path, slack_client=_MockClient()) @pytest.fixture(autouse=True) @@ -97,12 +112,23 @@ def _reset_holder(monkeypatch): def _read_log_lines(log_path: Path) -> list[dict]: + """Read ONLY inbound JSONL entries (filter to those with the + ``handled_status`` key). After ST2 the JSONL also contains + outbound entries with ``send_status`` instead — those have + their own dedicated test surface in + ``test_slack_dm_reply.py``; ST1 tests filter them out so the + inbound-filter assertions stay sharp. + """ if not log_path.exists(): return [] return [ - json.loads(line) - for line in log_path.read_text(encoding="utf-8").splitlines() - if line.strip() + entry + for entry in ( + json.loads(line) + for line in log_path.read_text(encoding="utf-8").splitlines() + if line.strip() + ) + if "handled_status" in entry ] @@ -355,18 +381,35 @@ async def test_malformed_payload_does_not_crash(handler, log_path): @pytest.mark.asyncio async def test_jsonl_one_json_per_line(handler, log_path): - """Multiple events → multiple lines, each parseable as JSON.""" + """Multiple events → multiple lines, each parseable as JSON. + + After ST2 each identified Joshua DM also writes an outbound + JSONL entry. 3 inbound events (2 Joshua + 1 USOMEONE) → + 5 lines total (3 inbound + 2 outbound for the Joshua pair). + The assertion is "all lines are valid JSON" — schema branches + on inbound (``received_at``) vs outbound (``sent_at``). + """ for i, user in enumerate([JOSHUA_ID, "USOMEONE", JOSHUA_ID]): await handler.handle_event( _make_payload(user=user, ts=f"170000000{i}.001") ) raw = log_path.read_text(encoding="utf-8") lines = raw.splitlines() - assert len(lines) == 3 + assert len(lines) == 5 # 3 inbound + 2 outbound (Joshua only) + inbound_count = 0 + outbound_count = 0 for line in lines: entry = json.loads(line) # parse-or-raise - for required in ("received_at", "user_id", "text", "handled_status"): - assert required in entry + if "handled_status" in entry: + for required in ("received_at", "user_id", "text"): + assert required in entry + inbound_count += 1 + else: + for required in ("sent_at", "channel_id", "send_status"): + assert required in entry + outbound_count += 1 + assert inbound_count == 3 + assert outbound_count == 2 @pytest.mark.asyncio diff --git a/tests/kora_cli/handlers/test_slack_dm_reply.py b/tests/kora_cli/handlers/test_slack_dm_reply.py new file mode 100644 index 000000000000..214a24430177 --- /dev/null +++ b/tests/kora_cli/handlers/test_slack_dm_reply.py @@ -0,0 +1,388 @@ +"""Tests for the ST2 outbound-reply integration in ``SlackDMHandler``. + +Covers: + - Joshua DM → inbound 'received' entry + outbound 'ok' entry, + in that order + - Bot token missing → outbound failed entry with + 'slack_client_not_configured' + reply_failed log emit + - SlackTransportError (e.g. 429 exhausted) → outbound failed entry + with 'transport:429' + - SlackAPIError (channel_not_found) → outbound failed entry with + 'slack_api:channel_not_found' + - Reply failure does NOT crash the handler (still returns ok) + - Filtered events (non-Joshua, bot, subtype) do NOT trigger reply + - PAUSED state drops do NOT trigger reply + - Echo format LOCKED: "Kora received: {text[:200]}" + - thread_ts = event.thread_ts if present, else event.ts + - Bot token NEVER appears in JSONL (security) +""" + +from __future__ import annotations + +import json +import logging +from pathlib import Path +from typing import Any, Dict, List +from unittest.mock import AsyncMock + +import pytest + +from kora_cli.clients.slack_client import ( + SlackAPIError, + SlackClientNotConfigured, + SlackTransportError, +) +from kora_cli.handlers.slack_dm_handler import ( + HANDLED_FILTERED_NON_JOSHUA, + HANDLED_RECEIVED, + JOSHUA_USER_ID_ENV, + SlackDMHandler, +) + + +JOSHUA_ID = "UJOSHUA01" + + +def _make_payload( + *, + user: str = JOSHUA_ID, + channel: str = "D01CHAN01", + text: str = "hello kora", + ts: str = "1700000000.001", + thread_ts: str | None = None, + channel_type: str = "im", + bot_id: str | None = None, + subtype: str | None = None, +) -> Dict[str, Any]: + event: Dict[str, Any] = { + "type": "message", + "user": user, + "channel": channel, + "channel_type": channel_type, + "text": text, + "ts": ts, + } + if thread_ts is not None: + event["thread_ts"] = thread_ts + if bot_id is not None: + event["bot_id"] = bot_id + if subtype is not None: + event["subtype"] = subtype + return {"type": "event_callback", "event": event} + + +def _read_lines(log_path: Path) -> List[Dict[str, Any]]: + if not log_path.exists(): + return [] + return [ + json.loads(line) + for line in log_path.read_text(encoding="utf-8").splitlines() + if line.strip() + ] + + +@pytest.fixture +def log_path(tmp_path) -> Path: + return tmp_path / "slack_dm_log.jsonl" + + +@pytest.fixture(autouse=True) +def _joshua_env(monkeypatch): + monkeypatch.setenv(JOSHUA_USER_ID_ENV, JOSHUA_ID) + + +@pytest.fixture(autouse=True) +def _reset_holder(monkeypatch): + from agent import operational_state_holder as h_mod + + monkeypatch.setattr(h_mod, "_HOLDER", None) + + +@pytest.fixture +def mock_client(): + """A SlackClient stand-in with a configurable post_dm AsyncMock.""" + + class _MockClient: + def __init__(self): + self.post_dm = AsyncMock( + return_value={"ok": True, "ts": "1700000001.999"} + ) + + return _MockClient() + + +# --------------------------------------------------------------------------- +# Happy path +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_joshua_dm_triggers_echo_reply_with_locked_format( + log_path, mock_client +): + handler = SlackDMHandler(log_path=log_path, slack_client=mock_client) + await handler.handle_event(_make_payload(text="ping")) + + mock_client.post_dm.assert_awaited_once() + call_kwargs = mock_client.post_dm.await_args.kwargs + assert call_kwargs["channel_id"] == "D01CHAN01" + # Echo format LOCKED. + assert call_kwargs["text"] == "Kora received: ping" + # thread_ts defaults to event.ts when event.thread_ts is absent. + assert call_kwargs["thread_ts"] == "1700000000.001" + + +@pytest.mark.asyncio +async def test_thread_ts_used_when_present(log_path, mock_client): + handler = SlackDMHandler(log_path=log_path, slack_client=mock_client) + await handler.handle_event( + _make_payload(thread_ts="1699999999.000", ts="1700000000.001") + ) + call_kwargs = mock_client.post_dm.await_args.kwargs + # Reply threads under the original thread, not the latest message. + assert call_kwargs["thread_ts"] == "1699999999.000" + + +@pytest.mark.asyncio +async def test_echo_text_truncated_at_200_chars(log_path, mock_client): + long_text = "x" * 5000 + handler = SlackDMHandler(log_path=log_path, slack_client=mock_client) + await handler.handle_event(_make_payload(text=long_text)) + sent_text = mock_client.post_dm.await_args.kwargs["text"] + # "Kora received: " is 15 chars, plus up to 200 of original. + assert sent_text.startswith("Kora received: ") + assert len(sent_text) == len("Kora received: ") + 200 + + +@pytest.mark.asyncio +async def test_jsonl_has_inbound_then_outbound_entry(log_path, mock_client): + handler = SlackDMHandler(log_path=log_path, slack_client=mock_client) + await handler.handle_event(_make_payload(text="hi")) + + entries = _read_lines(log_path) + assert len(entries) == 2 + + # Inbound first (received). + assert entries[0]["handled_status"] == HANDLED_RECEIVED + assert "received_at" in entries[0] + assert "sent_at" not in entries[0] + + # Outbound second (ok). + assert entries[1]["send_status"] == "ok" + assert "sent_at" in entries[1] + assert "received_at" not in entries[1] + assert entries[1]["slack_message_ts"] == "1700000001.999" + assert entries[1]["text"] == "Kora received: hi" + + +# --------------------------------------------------------------------------- +# Bot token missing → outbound failed entry +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_missing_bot_token_writes_failed_outbound_entry( + log_path, monkeypatch, caplog +): + """No KORA_SLACK_BOT_TOKEN → SlackClient lazy-construct fails → + outbound entry with failure_reason='slack_client_not_configured'.""" + caplog.set_level(logging.WARNING) + monkeypatch.delenv("KORA_SLACK_BOT_TOKEN", raising=False) + + # NO slack_client injected → handler tries to lazy-construct. + handler = SlackDMHandler(log_path=log_path) + result = await handler.handle_event(_make_payload(text="hi")) + assert result == {"ok": True} + + entries = _read_lines(log_path) + assert len(entries) == 2 + assert entries[1]["send_status"] == "failed" + assert entries[1]["failure_reason"] == "slack_client_not_configured" + assert entries[1]["slack_message_ts"] is None + + # reply_failed structured log emitted. + assert any( + "kora.slack_dm.reply_failed" in r.getMessage() + and "slack_client_not_configured" in r.getMessage() + for r in caplog.records + ) + + +# --------------------------------------------------------------------------- +# SlackTransportError → outbound failed entry +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_slack_transport_error_429_writes_failed_outbound( + log_path, mock_client, caplog +): + caplog.set_level(logging.WARNING) + mock_client.post_dm.side_effect = SlackTransportError( + "rate-limited after retry", last_status=429 + ) + + handler = SlackDMHandler(log_path=log_path, slack_client=mock_client) + result = await handler.handle_event(_make_payload()) + # Inbound handler still returns ok despite outbound failure. + assert result == {"ok": True} + + entries = _read_lines(log_path) + assert entries[1]["send_status"] == "failed" + assert entries[1]["failure_reason"] == "transport:429" + + +@pytest.mark.asyncio +async def test_slack_transport_error_500_writes_failed_outbound( + log_path, mock_client +): + mock_client.post_dm.side_effect = SlackTransportError( + "5xx after retry", last_status=500 + ) + handler = SlackDMHandler(log_path=log_path, slack_client=mock_client) + await handler.handle_event(_make_payload()) + [_, out] = _read_lines(log_path) + assert out["failure_reason"] == "transport:500" + + +@pytest.mark.asyncio +async def test_slack_transport_error_timeout_writes_failed_outbound( + log_path, mock_client +): + """Transport-level failure (no HTTP status) → reason='transport:'.""" + mock_client.post_dm.side_effect = SlackTransportError( + "timeout exhausted", last_status=None + ) + handler = SlackDMHandler(log_path=log_path, slack_client=mock_client) + await handler.handle_event(_make_payload()) + [_, out] = _read_lines(log_path) + assert out["failure_reason"] == "transport:SlackTransportError" + + +# --------------------------------------------------------------------------- +# SlackAPIError → outbound failed entry +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_slack_api_error_writes_failed_outbound( + log_path, mock_client +): + mock_client.post_dm.side_effect = SlackAPIError( + "channel_not_found", raw_response={"ok": False, "error": "channel_not_found"} + ) + handler = SlackDMHandler(log_path=log_path, slack_client=mock_client) + await handler.handle_event(_make_payload()) + [_, out] = _read_lines(log_path) + assert out["failure_reason"] == "slack_api:channel_not_found" + + +# --------------------------------------------------------------------------- +# Reply failure does NOT crash inbound handler +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_reply_failure_does_not_crash_handler(log_path, mock_client): + """Even an unexpected exception type (not in our error hierarchy) + must NOT crash handle_event. Outbound entry should still be written.""" + mock_client.post_dm.side_effect = RuntimeError("unexpected boom") + + handler = SlackDMHandler(log_path=log_path, slack_client=mock_client) + result = await handler.handle_event(_make_payload()) + assert result == {"ok": True} + [_, out] = _read_lines(log_path) + assert out["send_status"] == "failed" + # Stable reason code mapping. + assert out["failure_reason"] == "transport:RuntimeError" + + +# --------------------------------------------------------------------------- +# Filtered events do NOT trigger reply +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_non_joshua_no_reply_call(log_path, mock_client): + handler = SlackDMHandler(log_path=log_path, slack_client=mock_client) + await handler.handle_event(_make_payload(user="USOMEONE")) + mock_client.post_dm.assert_not_awaited() + # JSONL has only the inbound filtered entry — no outbound. + entries = _read_lines(log_path) + assert len(entries) == 1 + assert entries[0]["handled_status"] == HANDLED_FILTERED_NON_JOSHUA + + +@pytest.mark.asyncio +async def test_bot_message_no_reply_call(log_path, mock_client): + handler = SlackDMHandler(log_path=log_path, slack_client=mock_client) + await handler.handle_event(_make_payload(bot_id="B01")) + mock_client.post_dm.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_subtype_no_reply_call(log_path, mock_client): + handler = SlackDMHandler(log_path=log_path, slack_client=mock_client) + await handler.handle_event(_make_payload(subtype="message_changed")) + mock_client.post_dm.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_paused_state_no_reply_call(log_path, mock_client, monkeypatch): + """PAUSED state drops the message → reply MUST NOT fire (we don't + process Joshua's DM during a pause).""" + from agent.operational_state import OperationalState, PrimaryState + from agent.operational_state_holder import OperationalStateHolder + from agent import operational_state_holder as h_mod + + monkeypatch.setattr( + h_mod, + "_HOLDER", + OperationalStateHolder( + OperationalState(primary_state=PrimaryState.PAUSED) + ), + ) + + handler = SlackDMHandler(log_path=log_path, slack_client=mock_client) + await handler.handle_event(_make_payload()) + mock_client.post_dm.assert_not_awaited() + + +# --------------------------------------------------------------------------- +# Bot token NEVER in JSONL — security regression test +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_bot_token_never_in_jsonl(log_path, monkeypatch, mock_client): + """Diverse paths: success, transport-error, api-error, missing-token, + plus filtered events. After all paths, the bot token env value + MUST NOT appear in the JSONL.""" + token_marker = "xoxb-secret-bot-token-DO-NOT-LEAK" + monkeypatch.setenv("KORA_SLACK_BOT_TOKEN", token_marker) + + handler = SlackDMHandler(log_path=log_path, slack_client=mock_client) + + # Success path. + await handler.handle_event(_make_payload(text="hi", ts="1.001")) + + # Transport error path. + mock_client.post_dm.side_effect = SlackTransportError( + "rate-limited", last_status=429 + ) + await handler.handle_event(_make_payload(text="oops", ts="1.002")) + + # API error path. + mock_client.post_dm.side_effect = SlackAPIError( + "channel_not_found", raw_response={"ok": False, "error": "channel_not_found"} + ) + await handler.handle_event(_make_payload(text="bad", ts="1.003")) + + # Filtered (no reply call). + await handler.handle_event(_make_payload(user="USOMEONE", ts="1.004")) + + contents = log_path.read_text(encoding="utf-8") + assert token_marker not in contents, ( + "bot token env value appeared in JSONL — handler must NEVER " + "log secret material" + )