From 0dc293f4f774a2a17d50b7bb0c389f2559d317d4 Mon Sep 17 00:00:00 2001 From: Ben Barclay Date: Wed, 29 Jul 2026 12:03:01 -0700 Subject: [PATCH] fix(relay): coerce Slack behavior flags exactly as the native adapter does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both relay Slack knobs read their value through bool(), while the native adapter they mirror uses str(raw).strip().lower() in {"1","true","yes","on"}. A YAML-quoted string diverges: dm_top_level_threads_as_sessions: "false" → relay True, native False Non-empty strings are truthy, so the escape hatch is silently ignored in exactly the shape an operator writes to switch it OFF. reply_in_thread has the same defect and gates reply placement, session keying and run.py's progress resolver, so one quoted "false" misfires three ways. Route both through a shared _coerce_flag mirroring native's predicate. Real booleans pass through untouched; None falls back to the default. Contract §8 documents the accepted spellings. Tests: both knobs parametrized over the true/false spellings native accepts, plus the absent-key default. --- docs/relay-connector-contract.md | 11 +++++ gateway/relay/adapter.py | 29 ++++++++++--- .../relay/test_relay_slack_dm_streaming.py | 42 +++++++++++++++++++ 3 files changed, 77 insertions(+), 5 deletions(-) diff --git a/docs/relay-connector-contract.md b/docs/relay-connector-contract.md index 91ae62b69e0..184ce905e65 100644 --- a/docs/relay-connector-contract.md +++ b/docs/relay-connector-contract.md @@ -731,6 +731,10 @@ platforms: Resolution: nested `extra.` object wins → legacy flat key on `extra` honored as fallback → default. Source of truth: `RelayAdapter._effective_reply_in_thread` (`gateway/relay/adapter.py`). +Values coerce exactly as the native Slack adapter's do — `1/true/yes/on` +(case-insensitive, whitespace-trimmed) are ON, anything else is OFF — so a +YAML-quoted `"false"` turns a knob off rather than being read as a truthy +string. Current controls (Slack): @@ -745,6 +749,13 @@ thread-scoped, and in flat mode the send-side anchor strip guarantees the status anchor can never leak into reply placement. Semantics of the native key: see `website/docs/user-guide/messaging/slack.md`. +Thread-anchor resolution applies to EVERY send lane — text (`send`) and media +(`send_media`) alike — through one choke point +(`RelayAdapter._apply_slack_thread_anchor`). Media frames egress via the same +connector-side Slack sender, which threads on `metadata.thread_id` only, so an +attachment resolves its anchor identically to a text reply: promoted into +metadata in thread-per-message mode, stripped in flat mode. + Changes take effect on gateway restart; no connector involvement. --- diff --git a/gateway/relay/adapter.py b/gateway/relay/adapter.py index 7c61ff4763d..f4170d16444 100644 --- a/gateway/relay/adapter.py +++ b/gateway/relay/adapter.py @@ -344,10 +344,30 @@ class RelayAdapter(BasePlatformAdapter): sub = extra.get("slack") return sub if isinstance(sub, dict) else extra + @staticmethod + def _coerce_flag(raw: Any, default: bool) -> bool: + """Coerce an operator-supplied boolean exactly as native Slack does. + + Native SlackAdapter reads its behavior flags with + ``str(raw).strip().lower() in {"1","true","yes","on"}``, so a + YAML-quoted ``"false"`` — a shape operators write routinely — turns + the flag OFF. A bare ``bool()`` would read that same string as True + (non-empty string), silently ignoring the off switch. These knobs are + documented as native-parity mirrors, so they must coerce identically + or the parity claim only holds for unquoted YAML booleans. + """ + if raw is None: + return default + if isinstance(raw, bool): + return raw + return str(raw).strip().lower() in {"1", "true", "yes", "on"} + def _effective_reply_in_thread(self) -> bool: """Resolve the thread-per-message vs flat-DM mode for fronted Slack.""" try: - return bool(self._relay_slack_extra().get("reply_in_thread", True)) + return self._coerce_flag( + self._relay_slack_extra().get("reply_in_thread"), True + ) except Exception: # noqa: BLE001 - config shape is operator-owned return True @@ -363,10 +383,9 @@ class RelayAdapter(BasePlatformAdapter): legacy steer/queue posture, decoupled from reply_in_thread. """ try: - return bool( - self._relay_slack_extra().get( - "dm_top_level_threads_as_sessions", True - ) + return self._coerce_flag( + self._relay_slack_extra().get("dm_top_level_threads_as_sessions"), + True, ) except Exception: # noqa: BLE001 - config shape is operator-owned return True diff --git a/tests/gateway/relay/test_relay_slack_dm_streaming.py b/tests/gateway/relay/test_relay_slack_dm_streaming.py index a812f358ec3..9969b739575 100644 --- a/tests/gateway/relay/test_relay_slack_dm_streaming.py +++ b/tests/gateway/relay/test_relay_slack_dm_streaming.py @@ -341,6 +341,48 @@ async def test_media_caller_metadata_not_mutated(): assert caller_md == {"user_id": "U1"}, "caller metadata was mutated in place" +# --------------------------------------------------------------------------- +# Operator flags coerce exactly as the native Slack adapter's do. +# --------------------------------------------------------------------------- +@pytest.mark.parametrize( + "raw,expected", + [ + (False, False), + ("false", False), + ("False", False), + (" no ", False), + ("off", False), + ("0", False), + (True, True), + ("true", True), + ("yes", True), + ("on", True), + ("1", True), + ], +) +def test_relay_slack_flags_coerce_like_native(raw, expected): + """A YAML-quoted "false" must turn these knobs OFF, matching native's + str().strip().lower() predicate. A bare bool() would read any non-empty + string as True and silently ignore the operator's off switch.""" + adapter, _stub = _wire("D1", "dm") + adapter.config.extra = { + "slack": { + "reply_in_thread": raw, + "dm_top_level_threads_as_sessions": raw, + } + } + assert adapter._effective_reply_in_thread() is expected + assert adapter._dm_top_level_threads_as_sessions() is expected + + +def test_relay_slack_flags_default_true_when_absent(): + """Both knobs default ON when the operator sets nothing.""" + adapter, _stub = _wire("D1", "dm") + adapter.config.extra = {} + assert adapter._effective_reply_in_thread() is True + assert adapter._dm_top_level_threads_as_sessions() is True + + # --------------------------------------------------------------------------- # The status clear targets the same thread the heartbeat set. # ---------------------------------------------------------------------------