mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-21 02:05:45 +00:00
Fix require_mention gating on whitespace-only bot_login/mention_login (#4055)
github.bot_login and trigger.mention_login were read raw in the require_mention precedence chain, so a whitespace-only value (e.g. " ") never fell through to the working fallback the chain documents - Python truthiness lets " " win an `or` chain over agent.name or the operator default. The third link (channels.github.default_mention_login) was already correctly normalized and pinned by test_operator_default_blank_string_treated_as_none; this closes the gap for the other two by normalizing GitHubTriggerConfig.mention_login and GitHubAgentConfig.bot_login once, at the config layer, via a field_validator mirroring the pattern already used elsewhere in the config package.
This commit is contained in:
@@ -201,6 +201,14 @@ async def fanout_event(
|
||||
# ``coder`` with ``require_mention: true`` and no per-trigger or
|
||||
# per-agent override silently required ``@coder`` mentions instead
|
||||
# of ``@deerflow-bot``.
|
||||
#
|
||||
# ``github.bot_login`` is normalized (whitespace-only -> None) by
|
||||
# ``GitHubAgentConfig``'s field validator, so this ``or`` chain
|
||||
# correctly falls through a misconfigured ``bot_login: " "``
|
||||
# instead of comparing mentions against a literal whitespace
|
||||
# string. ``operator_default_mention_login`` is a plain function
|
||||
# argument (not a validated model field), so it is normalized here
|
||||
# explicitly.
|
||||
operator_default = (operator_default_mention_login or "").strip() or None
|
||||
default_mention_login = github.bot_login or operator_default or agent.name
|
||||
fire, reason = event_should_fire(event, payload, trigger, default_mention_login)
|
||||
|
||||
@@ -186,6 +186,11 @@ def event_should_fire(
|
||||
return True, f"allow_authors:{author}"
|
||||
|
||||
if trigger.require_mention:
|
||||
# ``trigger.mention_login`` is normalized (whitespace-only -> None)
|
||||
# by ``GitHubTriggerConfig``'s field validator, so this ``or`` falls
|
||||
# through a misconfigured ``mention_login: " "`` to
|
||||
# ``default_mention_login`` instead of gating on a literal
|
||||
# whitespace string that no real ``@mention`` could ever match.
|
||||
login = trigger.mention_login or default_mention_login
|
||||
body = _comment_body(event, payload)
|
||||
# Boundary-aware @-mention match: ``@deerflow`` must NOT match
|
||||
|
||||
@@ -13,7 +13,7 @@ from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
import yaml
|
||||
from pydantic import BaseModel, Field, model_validator
|
||||
from pydantic import BaseModel, Field, field_validator, model_validator
|
||||
|
||||
from deerflow.config.paths import get_paths
|
||||
from deerflow.runtime.user_context import get_effective_user_id
|
||||
@@ -24,6 +24,25 @@ SOUL_FILENAME = "SOUL.md"
|
||||
AGENT_NAME_PATTERN = re.compile(r"^[A-Za-z0-9-]+$")
|
||||
|
||||
|
||||
def _blank_to_none(value: str | None) -> str | None:
|
||||
"""Normalize a whitespace-only string to ``None``; leave real values untouched.
|
||||
|
||||
A whitespace-only string (e.g. ``" "``) is truthy in Python, so an
|
||||
unstripped ``value or fallback`` expression never falls through to the
|
||||
fallback. The ``require_mention`` precedence chain (``trigger.mention_login``
|
||||
-> ``github.bot_login`` -> ``channels.github.default_mention_login`` ->
|
||||
``agent.name``, see AGENTS.md) relies on exactly that fallthrough, so both
|
||||
of the config-sourced links are normalized here, once, at the model layer
|
||||
— every reader downstream (today's and any future one) sees an honest
|
||||
"unset" instead of a literal whitespace string that can never match a
|
||||
real ``@mention``.
|
||||
"""
|
||||
if value is None:
|
||||
return None
|
||||
stripped = value.strip()
|
||||
return stripped or None
|
||||
|
||||
|
||||
class GitHubTriggerConfig(BaseModel):
|
||||
"""Per-event trigger filter inside a :class:`GitHubBinding`."""
|
||||
|
||||
@@ -38,9 +57,17 @@ class GitHubTriggerConfig(BaseModel):
|
||||
# talk to the bot without typing the handle every time.
|
||||
allow_authors: list[str] = Field(default_factory=list)
|
||||
# Override the global default bot mention login for this trigger only.
|
||||
# Useful when one agent answers as @bot-a and another as @bot-b.
|
||||
# Useful when one agent answers as @bot-a and another as @bot-b. A
|
||||
# whitespace-only value is normalized to None (see ``_blank_to_none``) so
|
||||
# it is treated as unset and falls through to ``github.bot_login`` instead
|
||||
# of being compared against literally.
|
||||
mention_login: str | None = None
|
||||
|
||||
@field_validator("mention_login")
|
||||
@classmethod
|
||||
def _normalize_mention_login(cls, value: str | None) -> str | None:
|
||||
return _blank_to_none(value)
|
||||
|
||||
|
||||
class GitHubBinding(BaseModel):
|
||||
"""One (agent, repo) binding with per-event trigger overrides."""
|
||||
@@ -71,6 +98,8 @@ class GitHubAgentConfig(BaseModel):
|
||||
# ``mention_login`` the agent uses for trigger matching. None means
|
||||
# "fall back to mention_login / agent name", which is fine when those
|
||||
# match the bot identity, but should be set explicitly when they differ.
|
||||
# A whitespace-only value is normalized to None (see ``_blank_to_none``)
|
||||
# so it is treated as unset and falls through the rest of the chain.
|
||||
bot_login: str | None = None
|
||||
# Override the default github-channel ``recursion_limit`` (250). GitHub
|
||||
# runs are autonomous and long-running by nature — clone, explore, edit,
|
||||
@@ -87,6 +116,11 @@ class GitHubAgentConfig(BaseModel):
|
||||
# never fires from a webhook, even if it has a ``github:`` block.
|
||||
bindings: list[GitHubBinding] = Field(default_factory=list)
|
||||
|
||||
@field_validator("bot_login")
|
||||
@classmethod
|
||||
def _normalize_bot_login(cls, value: str | None) -> str | None:
|
||||
return _blank_to_none(value)
|
||||
|
||||
@model_validator(mode="after")
|
||||
def _unique_binding_repos(self) -> "GitHubAgentConfig":
|
||||
"""Reject duplicate ``repo`` values across ``bindings``.
|
||||
|
||||
@@ -798,6 +798,95 @@ async def test_operator_default_blank_string_treated_as_none(base_dir: Path) ->
|
||||
assert result["fired_agents"] == ["assistant"], result
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_bot_login_whitespace_only_treated_as_none(base_dir: Path) -> None:
|
||||
"""A whitespace-only ``github.bot_login`` must not silently become the
|
||||
mention-gating handle.
|
||||
|
||||
AGENTS.md documents the whole ``require_mention`` precedence chain
|
||||
(``trigger.mention_login`` -> ``github.bot_login`` ->
|
||||
``channels.github.default_mention_login`` -> ``agent.name``) as treating
|
||||
whitespace-only defaults as unset. A misconfigured ``bot_login: " "``
|
||||
(e.g. a YAML templating slip) is truthy in Python, so an unstripped
|
||||
``github.bot_login or operator_default or agent.name`` never falls
|
||||
through to the working ``agent.name`` fallback — every legitimate
|
||||
``@assistant`` mention is silently rejected and the trigger can never
|
||||
fire again until an operator notices and fixes the typo.
|
||||
"""
|
||||
bus = MessageBus()
|
||||
_write_agent(
|
||||
base_dir,
|
||||
"default",
|
||||
"assistant",
|
||||
{
|
||||
"name": "assistant",
|
||||
"github": {
|
||||
"bot_login": " ", # whitespace-only — must be treated as unset
|
||||
"bindings": [
|
||||
{"repo": "a/b", "triggers": {"issue_comment": {"require_mention": True}}},
|
||||
],
|
||||
},
|
||||
},
|
||||
)
|
||||
payload = {
|
||||
"action": "created",
|
||||
"issue": {"number": 7, "pull_request": {"url": "..."}},
|
||||
"comment": {"body": "hey @assistant please", "user": {"login": "alice"}},
|
||||
"repository": {"full_name": "a/b"},
|
||||
"sender": {"login": "alice"},
|
||||
}
|
||||
# Whitespace-only bot_login → falls through to the agent.name fallback.
|
||||
result = await fanout_event(bus, "issue_comment", "del-blank-bot-login", payload)
|
||||
assert result["fired_agents"] == ["assistant"], result
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_trigger_mention_login_whitespace_only_treated_as_none(base_dir: Path) -> None:
|
||||
"""A whitespace-only per-trigger ``mention_login`` must not silently
|
||||
become the mention-gating handle either.
|
||||
|
||||
Same contract as ``test_bot_login_whitespace_only_treated_as_none``, one
|
||||
link higher in the precedence chain: ``event_should_fire`` reads
|
||||
``trigger.mention_login`` first. A misconfigured
|
||||
``mention_login: " "`` is truthy, so an unstripped
|
||||
``trigger.mention_login or default_mention_login`` never falls through
|
||||
to the agent's real ``github.bot_login`` handle.
|
||||
"""
|
||||
bus = MessageBus()
|
||||
_write_agent(
|
||||
base_dir,
|
||||
"default",
|
||||
"coder",
|
||||
{
|
||||
"name": "coder",
|
||||
"github": {
|
||||
"bot_login": "deerflow-bot", # the real, working fallback handle
|
||||
"bindings": [
|
||||
{
|
||||
"repo": "a/b",
|
||||
"triggers": {
|
||||
"issue_comment": {
|
||||
"require_mention": True,
|
||||
"mention_login": " ", # whitespace-only — must be treated as unset
|
||||
}
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
)
|
||||
payload = {
|
||||
"action": "created",
|
||||
"issue": {"number": 7, "pull_request": {"url": "..."}},
|
||||
"comment": {"body": "hey @deerflow-bot please look", "user": {"login": "alice"}},
|
||||
"repository": {"full_name": "a/b"},
|
||||
"sender": {"login": "alice"},
|
||||
}
|
||||
# Whitespace-only trigger.mention_login → falls through to github.bot_login.
|
||||
result = await fanout_event(bus, "issue_comment", "del-blank-trigger-mention", payload)
|
||||
assert result["fired_agents"] == ["coder"], result
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Multiple agents
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user