fix(relay): coerce string unfurl knobs and disable Slack draft streaming
Live staging (Coatue Slack): - hermes config set / Railway knobs persist "true" as a string; bots that omit unfurl_links do NOT inherit the human default, so dropping the string looked like suppression. - chat.startStream cannot carry unfurl_*. Native SlackAdapter already falls back to chat.postMessage; the relay now matches.
This commit is contained in:
committed by
Ben Barclay
parent
db00793db7
commit
634d9c3f4c
@@ -317,10 +317,23 @@ class RelayAdapter(BasePlatformAdapter):
|
||||
if chat_id is not None
|
||||
else self.descriptor
|
||||
)
|
||||
return (
|
||||
if not (
|
||||
desc.supports_draft_streaming
|
||||
and "draft" in (desc.supported_ops or ())
|
||||
)
|
||||
):
|
||||
return False
|
||||
# Slack chat.*Stream has no unfurl_links / unfurl_media. Native
|
||||
# SlackAdapter already refuses streaming when those knobs are set
|
||||
# so chat.postMessage can carry them. Mirror that here or a
|
||||
# configured true never reaches Slack (bot default = no preview).
|
||||
platform = None
|
||||
if chat_id is not None:
|
||||
platform = self._platform_by_chat.get(str(chat_id))
|
||||
if platform is None:
|
||||
platform = getattr(desc, "platform", None)
|
||||
if self._slack_unfurl_hints(platform):
|
||||
return False
|
||||
return True
|
||||
|
||||
def stream_is_message_for_chat(self, chat_id: str) -> bool:
|
||||
"""Per-chat stream-is-the-message semantic (review r2, finding 2).
|
||||
@@ -1098,8 +1111,27 @@ class RelayAdapter(BasePlatformAdapter):
|
||||
hints: Dict[str, bool] = {}
|
||||
for knob in ("unfurl_links", "unfurl_media"):
|
||||
val = extra.get(knob)
|
||||
if val is None:
|
||||
continue
|
||||
# Railway / `hermes config set` write YAML strings ("true"/"false").
|
||||
# A Slack bot that omits the fields does NOT get human-default
|
||||
# previews — so a string "true" that we drop looks like
|
||||
# suppression. Coerce the same way as reply_in_thread; still drop
|
||||
# junk (empty, 0, "maybe") so omitted stays omitted.
|
||||
if isinstance(val, bool):
|
||||
hints[knob] = val
|
||||
continue
|
||||
if isinstance(val, str) and val.strip().lower() in {
|
||||
"1",
|
||||
"0",
|
||||
"true",
|
||||
"false",
|
||||
"yes",
|
||||
"no",
|
||||
"on",
|
||||
"off",
|
||||
}:
|
||||
hints[knob] = val.strip().lower() in {"1", "true", "yes", "on"}
|
||||
return hints or None
|
||||
|
||||
def _stamp_slack_session_thread(self, event) -> None:
|
||||
|
||||
@@ -88,8 +88,16 @@ class TestUnfurlHints:
|
||||
a = _slack_adapter({"slack": {}})
|
||||
assert a._slack_unfurl_hints("slack") is None
|
||||
|
||||
def test_non_bool_values_dropped(self):
|
||||
a = _slack_adapter({"slack": {"unfurl_links": "false", "unfurl_media": 0}})
|
||||
def test_string_bools_from_config_set_are_coerced(self):
|
||||
# Railway knobs / `hermes config set` persist YAML strings.
|
||||
a = _slack_adapter({"slack": {"unfurl_links": "true", "unfurl_media": "false"}})
|
||||
assert a._slack_unfurl_hints("slack") == {
|
||||
"unfurl_links": True,
|
||||
"unfurl_media": False,
|
||||
}
|
||||
|
||||
def test_junk_values_dropped(self):
|
||||
a = _slack_adapter({"slack": {"unfurl_links": "maybe", "unfurl_media": 0}})
|
||||
assert a._slack_unfurl_hints("slack") is None
|
||||
|
||||
def test_flat_legacy_key_fallback(self):
|
||||
@@ -155,3 +163,40 @@ class TestSendForPlatformStampsUnfurl:
|
||||
await a.send_for_platform(P.SLACK, "C123", "brief")
|
||||
assert "unfurl_links" not in a._transport.sent["metadata"]
|
||||
assert "unfurl_media" not in a._transport.sent["metadata"]
|
||||
|
||||
class TestUnfurlDisablesDraftStreaming:
|
||||
def test_explicit_unfurl_disables_slack_draft_stream(self):
|
||||
a = RelayAdapter(
|
||||
PlatformConfig(extra={"slack": {"unfurl_links": True}}),
|
||||
make_desc(
|
||||
platform="slack",
|
||||
supports_draft_streaming=True,
|
||||
supported_ops=("send", "draft"),
|
||||
),
|
||||
transport=_CaptureTransport(),
|
||||
)
|
||||
assert a.supports_draft_streaming() is False
|
||||
|
||||
def test_omitted_unfurl_keeps_slack_draft_stream(self):
|
||||
a = RelayAdapter(
|
||||
PlatformConfig(extra={"slack": {}}),
|
||||
make_desc(
|
||||
platform="slack",
|
||||
supports_draft_streaming=True,
|
||||
supported_ops=("send", "draft"),
|
||||
),
|
||||
transport=_CaptureTransport(),
|
||||
)
|
||||
assert a.supports_draft_streaming() is True
|
||||
|
||||
def test_string_true_also_disables_stream(self):
|
||||
a = RelayAdapter(
|
||||
PlatformConfig(extra={"slack": {"unfurl_links": "true"}}),
|
||||
make_desc(
|
||||
platform="slack",
|
||||
supports_draft_streaming=True,
|
||||
supported_ops=("send", "draft"),
|
||||
),
|
||||
transport=_CaptureTransport(),
|
||||
)
|
||||
assert a.supports_draft_streaming() is False
|
||||
|
||||
Reference in New Issue
Block a user