mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 13:16:24 +00:00
fix(buzz): a single mention_free_channels ID was split into characters, a numeric one stopped the connector (#6776)
* fix(buzz): accept a single mention-free channel id `_mention_free` iterated the configured value directly, so one unquoted UUID became a set of its characters and a numeric or boolean scalar raised TypeError while the channel was being constructed. Both shapes are documented as valid for the sibling Discord connector. Parse the key through a dedicated helper that follows the Discord convention: a scalar is a single entry, list entries are kept verbatim, and an unusable entry never matches so `require_mention` stays in force. * fix(channels): share scalar-or-list channel ID parsing * docs(channels): keep guidance within CI size budget --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
parent
20887e465c
commit
9b2151a3a8
@ -68,7 +68,7 @@ The cached value is reused for both the blocking (`runs.wait`) and streaming (`_
|
||||
- No public IP, OAuth callback URL, or provider webhook route is required by the current implementation.
|
||||
- Telegram binds via deep-link `/start <code>` over long polling; Slack, Discord, Feishu/Lark, DingTalk, WeChat, and WeCom use `/connect <code>` through their outbound workers.
|
||||
- WeChat `polling_timeout`, `polling_retry_delay`, `qrcode_poll_interval`, and `qrcode_poll_timeout` require positive finite seconds; invalid values use defaults to prevent hot loops or endless sleeps.
|
||||
- `allowed_users.py` shares QQ/WeChat/Telegram parsing: unset/null/empty lists/blank strings allow all; invalid entries warn; all-invalid denies all with an error. QQ/WeChat opt into `warn_on_scalar_separators` without splitting IDs. WeChat converts integral numbers to text and gates before media downloads.
|
||||
- `allowed_users.py`: QQ/WeChat/Telegram empty allows all; invalid warns; all-invalid denies all. WeChat coerces integral IDs and gates downloads. Buzz/Discord `parse_channel_ids`: trims IDs; empty = no exemption; no splitting/mapping keys. Buzz/QQ/WeChat warn on scalar separators.
|
||||
- WeCom serializes `start()` and `stop()` for each channel instance. The SDK `connect()` task covers connection setup only; after the handshake, the SDK owns a separate receive task. Shutdown cancels an in-progress connection attempt and awaits the SDK's actual asynchronous receive-task/socket cleanup before releasing lifecycle state or allowing a restart. Cancellation of `stop()` still propagates, but only after owned cleanup finishes and lifecycle references are cleared; real connection failures remain reported by `_on_ws_task_done`.
|
||||
- WeCom outbound content is capped at the protocol's 20480 UTF-8 bytes: stream replies clip on a character boundary with a truncation marker (one stream carries the whole reply and cannot split mid-way), while proactive pushes split into at most 10 sequential markdown messages per push, with the remaining tail clipped and marker-terminated. Both paths measure bytes, not characters. A per-chat send lock serializes each split batch end to end, because manager workers run concurrently and two long pushes to the same chat would otherwise interleave chunks; locks are reference-counted and reclaimed once no sender holds or waits on them, so the registry does not grow for the life of the Gateway.
|
||||
- Frontend APIs: `GET /api/channels/providers`, `GET /api/channels/connections`, `POST /api/channels/{provider}/connect`, and `DELETE /api/channels/connections/{connection_id}`.
|
||||
|
||||
@ -1,4 +1,4 @@
|
||||
"""Shared fail-closed parsing for channel user allowlists."""
|
||||
"""Shared scalar-or-list parsing for channel ID configuration."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
@ -7,6 +7,52 @@ from collections.abc import Callable, Hashable
|
||||
from typing import Any
|
||||
|
||||
|
||||
def _parse_id_entries(
|
||||
value: Any,
|
||||
*,
|
||||
logger: logging.Logger,
|
||||
channel_name: str,
|
||||
key_name: str,
|
||||
valid_id_name: str,
|
||||
warn_on_scalar_separators: bool,
|
||||
) -> list[Any]:
|
||||
"""Normalize config shape without splitting scalar IDs or iterating mappings."""
|
||||
if value is None or (isinstance(value, str) and not value.strip()):
|
||||
return []
|
||||
if warn_on_scalar_separators and isinstance(value, str) and ("," in value or any(char.isspace() for char in value.strip())):
|
||||
logger.warning(
|
||||
"[%s] %s is a scalar string containing separators; treating it as one literal %s. Use a YAML list for multiple IDs",
|
||||
channel_name,
|
||||
key_name,
|
||||
valid_id_name,
|
||||
)
|
||||
return list(value) if isinstance(value, (list, tuple, set, frozenset)) else [value]
|
||||
|
||||
|
||||
def parse_channel_ids(
|
||||
value: Any,
|
||||
*,
|
||||
logger: logging.Logger,
|
||||
channel_name: str,
|
||||
key_name: str,
|
||||
warn_on_scalar_separators: bool = False,
|
||||
) -> frozenset[str]:
|
||||
"""Return trimmed literal channel IDs; empty config grants no exemptions.
|
||||
|
||||
These IDs are compared as strings by the connector. An unusable literal
|
||||
never matches, so it leaves the connector's mention requirement in force.
|
||||
"""
|
||||
entries = _parse_id_entries(
|
||||
value,
|
||||
logger=logger,
|
||||
channel_name=channel_name,
|
||||
key_name=key_name,
|
||||
valid_id_name="channel id",
|
||||
warn_on_scalar_separators=warn_on_scalar_separators,
|
||||
)
|
||||
return frozenset(text for text in (str(entry).strip() for entry in entries) if text)
|
||||
|
||||
|
||||
def parse_allowed_users[UserId: Hashable](
|
||||
allowed_users: Any,
|
||||
*,
|
||||
@ -26,15 +72,14 @@ def parse_allowed_users[UserId: Hashable](
|
||||
String-ID channels can opt into a scalar-separator warning without splitting
|
||||
the ID; surrounding whitespace alone does not trigger it.
|
||||
"""
|
||||
if allowed_users is None or (isinstance(allowed_users, str) and not allowed_users.strip()):
|
||||
return None
|
||||
if warn_on_scalar_separators and isinstance(allowed_users, str) and ("," in allowed_users or any(char.isspace() for char in allowed_users.strip())):
|
||||
logger.warning(
|
||||
"[%s] allowed_users is a scalar string containing separators; treating it as one literal %s. Use a YAML list for multiple IDs",
|
||||
channel_name,
|
||||
valid_id_name,
|
||||
)
|
||||
entries = list(allowed_users) if isinstance(allowed_users, (list, tuple, set, frozenset)) else [allowed_users]
|
||||
entries = _parse_id_entries(
|
||||
allowed_users,
|
||||
logger=logger,
|
||||
channel_name=channel_name,
|
||||
key_name="allowed_users",
|
||||
valid_id_name=valid_id_name,
|
||||
warn_on_scalar_separators=warn_on_scalar_separators,
|
||||
)
|
||||
if not entries:
|
||||
return None
|
||||
|
||||
|
||||
@ -49,6 +49,7 @@ from typing import Any
|
||||
from urllib.parse import urlparse
|
||||
|
||||
from app.channels import buzz_nostr
|
||||
from app.channels.allowed_users import parse_channel_ids
|
||||
from app.channels.base import Channel
|
||||
from app.channels.buzz_seen_events import BuzzSeenEventStore
|
||||
from app.channels.commands import is_known_channel_command
|
||||
@ -249,6 +250,17 @@ def _is_transient_close(reason: str) -> bool:
|
||||
return not any(marker in text for marker in _PERMANENT_CLOSE_MARKERS)
|
||||
|
||||
|
||||
def _parse_mention_free_channels(mention_free_channels: Any) -> frozenset[str]:
|
||||
"""Parse Buzz mention exemptions without splitting scalar channel IDs."""
|
||||
return parse_channel_ids(
|
||||
mention_free_channels,
|
||||
logger=logger,
|
||||
channel_name="buzz",
|
||||
key_name="mention_free_channels",
|
||||
warn_on_scalar_separators=True,
|
||||
)
|
||||
|
||||
|
||||
class BuzzChannel(Channel):
|
||||
_connect: Any = None # test seam: async callable returning an async-context-manager transport
|
||||
|
||||
@ -267,7 +279,7 @@ class BuzzChannel(Channel):
|
||||
self._keys: buzz_nostr.NostrKeys | None = None # parsed in start() so coincurve stays lazy
|
||||
self._allowed_users = {buzz_nostr.parse_pubkey(v) for v in config.get("allowed_users", []) or []}
|
||||
self._require_mention = bool(config.get("require_mention", True))
|
||||
self._mention_free = {str(c) for c in config.get("mention_free_channels", []) or []}
|
||||
self._mention_free: frozenset[str] = _parse_mention_free_channels(config.get("mention_free_channels"))
|
||||
self._channel_meta: dict[str, dict[str, Any]] = {}
|
||||
self._stream_targets: dict[tuple[str, str | None], str] = {}
|
||||
self._stream_tails: dict[tuple[str, str | None], list[str]] = {} # overflow chunk ids beyond chunk 0, per conversation
|
||||
|
||||
@ -11,6 +11,7 @@ from concurrent.futures import Future
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
from app.channels.allowed_users import parse_channel_ids
|
||||
from app.channels.base import Channel
|
||||
from app.channels.commands import is_known_channel_command
|
||||
from app.channels.connection_identity import attach_connection_identity
|
||||
@ -81,10 +82,7 @@ def _parse_allowed_channels(allowed_channels: Any) -> frozenset[str]:
|
||||
the entries are kept verbatim rather than validated — an unreadable ID just
|
||||
never matches, which leaves ``mention_only`` in force (fail-closed).
|
||||
"""
|
||||
if allowed_channels is None or (isinstance(allowed_channels, str) and not allowed_channels.strip()):
|
||||
return frozenset()
|
||||
entries = list(allowed_channels) if isinstance(allowed_channels, list | tuple | set) else [allowed_channels]
|
||||
return frozenset(text for text in (str(entry).strip() for entry in entries) if text)
|
||||
return parse_channel_ids(allowed_channels, logger=logger, channel_name="Discord", key_name="allowed_channels")
|
||||
|
||||
|
||||
class DiscordChannel(Channel):
|
||||
|
||||
@ -5,7 +5,7 @@ from typing import Any
|
||||
|
||||
import pytest
|
||||
|
||||
from app.channels.allowed_users import parse_allowed_users
|
||||
from app.channels.allowed_users import parse_allowed_users, parse_channel_ids
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@ -81,3 +81,48 @@ def test_opted_in_warning_only_checks_scalar_separators(allowed_users, expected,
|
||||
|
||||
assert result == expected
|
||||
assert caplog.records == []
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"value, expected",
|
||||
[
|
||||
(None, frozenset()),
|
||||
(" \t\n ", frozenset()),
|
||||
([], frozenset()),
|
||||
((), frozenset()),
|
||||
(set(), frozenset()),
|
||||
(frozenset(), frozenset()),
|
||||
(" 123 ", frozenset({"123"})),
|
||||
(123, frozenset({"123"})),
|
||||
(["123", " 456 ", "", "123"], frozenset({"123", "456"})),
|
||||
(("123", "456"), frozenset({"123", "456"})),
|
||||
({"123", "456"}, frozenset({"123", "456"})),
|
||||
(frozenset({"123", "456"}), frozenset({"123", "456"})),
|
||||
({"123": True}, frozenset({"{'123': True}"})),
|
||||
([None, True], frozenset({"None", "True"})),
|
||||
(["123,456", "789 012"], frozenset({"123,456", "789 012"})),
|
||||
],
|
||||
)
|
||||
def test_channel_ids_preserve_literal_entries_and_empty_policy(value, expected, caplog):
|
||||
result = parse_channel_ids(value, logger=logger, channel_name="buzz", key_name="mention_free_channels", warn_on_scalar_separators=True)
|
||||
|
||||
assert result == expected
|
||||
assert caplog.records == []
|
||||
|
||||
|
||||
@pytest.mark.parametrize("separator", [",", ", ", " ", "\t", "\n", "\u2003"])
|
||||
@pytest.mark.parametrize("channel_name, key_name, warn", [("buzz", "mention_free_channels", True), ("Discord", "allowed_channels", False)])
|
||||
def test_channel_id_warning_names_config_key_and_preserves_scalar(channel_name, key_name, warn, separator, caplog):
|
||||
literal_id = f"123{separator}456"
|
||||
with caplog.at_level(logging.WARNING, logger=logger.name):
|
||||
result = parse_channel_ids(literal_id, logger=logger, channel_name=channel_name, key_name=key_name, warn_on_scalar_separators=warn)
|
||||
|
||||
assert result == frozenset({literal_id})
|
||||
assert len(caplog.records) == int(warn)
|
||||
if warn:
|
||||
warning = caplog.records[0]
|
||||
assert warning.levelno == logging.WARNING
|
||||
assert f"[{channel_name}] {key_name}" in warning.message
|
||||
assert "one literal channel id" in warning.message
|
||||
assert "YAML list" in warning.message
|
||||
assert "123" not in warning.message and "456" not in warning.message
|
||||
|
||||
@ -63,6 +63,41 @@ def test_config_parsing_normalizes_allowlist_and_defaults():
|
||||
assert ch._relay_url == "wss://buzz.example.com"
|
||||
|
||||
|
||||
def test_config_mention_free_channel_accepts_a_single_id():
|
||||
"""One UUID, quoted or unquoted, is one entry -- not the characters of it.
|
||||
|
||||
A scalar was iterated as if it were a list, so the exemption matched nothing:
|
||||
the channel the operator just made mention-free kept requiring a mention, which
|
||||
looks exactly like a deaf bot. Discord documents and parses the same shape.
|
||||
"""
|
||||
assert _channel(mention_free_channels=CHANNEL)._mention_free == frozenset({CHANNEL})
|
||||
assert _channel(mention_free_channels=f" {CHANNEL} ")._mention_free == frozenset({CHANNEL})
|
||||
assert CHANNEL in _channel(mention_free_channels=CHANNEL)._mention_free
|
||||
|
||||
|
||||
def test_config_mention_free_channel_accepts_a_non_string_scalar():
|
||||
"""An unquoted value YAML typed as a number must not abort channel setup."""
|
||||
assert _channel(mention_free_channels=136852)._mention_free == frozenset({"136852"})
|
||||
|
||||
|
||||
def test_config_mention_free_channel_empty_shapes_mean_no_exemption():
|
||||
for value in (None, "", " ", [], frozenset()):
|
||||
assert _channel(mention_free_channels=value)._mention_free == frozenset()
|
||||
|
||||
|
||||
def test_config_mention_free_channel_keeps_list_entries_verbatim():
|
||||
ch = _channel(mention_free_channels=[CHANNEL, f" {CHANNEL_B} ", ""])
|
||||
assert ch._mention_free == frozenset({CHANNEL, CHANNEL_B})
|
||||
|
||||
|
||||
def test_config_mention_free_channel_scalar_with_separators_warns(caplog):
|
||||
"""A comma/space-joined scalar is one literal id, and says so rather than going quiet."""
|
||||
with caplog.at_level(logging.WARNING, logger="app.channels.buzz"):
|
||||
ch = _channel(mention_free_channels=f"{CHANNEL}, {CHANNEL_B}")
|
||||
assert ch._mention_free == frozenset({f"{CHANNEL}, {CHANNEL_B}"})
|
||||
assert "treating it as one literal channel id" in caplog.text
|
||||
|
||||
|
||||
def test_config_rejects_non_websocket_relay_url():
|
||||
with pytest.raises(ValueError):
|
||||
_channel(relay_url="https://buzz.example.com")
|
||||
|
||||
@ -36,6 +36,20 @@ def test_discord_channel_init() -> None:
|
||||
assert channel.name == "discord"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("collection", [list, tuple, set, frozenset])
|
||||
def test_allowed_channels_accepts_all_id_collections(collection):
|
||||
channel = DiscordChannel(bus=MessageBus(), config={"allowed_channels": collection([" 456 ", "789", ""])})
|
||||
|
||||
assert channel._allowed_channels == frozenset({"456", "789"})
|
||||
|
||||
|
||||
@pytest.mark.parametrize("value", [None, "", " \t ", [], (), set(), frozenset()])
|
||||
def test_empty_allowed_channels_gives_no_mention_exemption(value):
|
||||
channel = DiscordChannel(bus=MessageBus(), config={"allowed_channels": value})
|
||||
|
||||
assert channel._allowed_channels == frozenset()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_start_without_discord_module_points_at_the_extra(caplog) -> None:
|
||||
"""discord.py ships in the optional ``discord`` extra, so the missing-dependency
|
||||
|
||||
@ -3211,7 +3211,8 @@ run_ownership:
|
||||
# # DENY-BY-DEFAULT (unlike other channels): empty
|
||||
# # means nobody, and DeerFlow logs a startup warning.
|
||||
# require_mention: true # @mention needed in channels
|
||||
# mention_free_channels: [] # channel UUIDs that respond to every message
|
||||
# mention_free_channels: [] # Channel UUIDs that respond to every message:
|
||||
# # one UUID (quoted or unquoted) or a YAML list.
|
||||
|
||||
# ============================================================================
|
||||
# Guardrails Configuration
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user