mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-26 17:38:36 +00:00
fix(gateway): quiet Slack missing_scope channel directory fallback
Treat Slack users.conversations missing_scope as an expected limited-scope app condition and fall back to session history without recurring warnings. Add tests for not-ok and SlackApiError-like missing_scope responses.
This commit is contained in:
parent
0eb5cb5e07
commit
c1529e58da
2 changed files with 57 additions and 9 deletions
|
|
@ -244,13 +244,29 @@ def _build_discord(adapter) -> List[Dict[str, str]]:
|
|||
return channels
|
||||
|
||||
|
||||
def _slack_api_error_code(error: Exception) -> Optional[str]:
|
||||
"""Return Slack Web API error code from SlackApiError-like exceptions."""
|
||||
response = getattr(error, "response", None)
|
||||
if isinstance(response, dict):
|
||||
value = response.get("error")
|
||||
return str(value) if value else None
|
||||
if response is not None:
|
||||
try:
|
||||
value = response.get("error")
|
||||
return str(value) if value else None
|
||||
except Exception:
|
||||
pass
|
||||
return None
|
||||
|
||||
|
||||
async def _build_slack(adapter) -> List[Dict[str, Any]]:
|
||||
"""List Slack channels the bot has joined across all workspaces.
|
||||
|
||||
Uses ``users.conversations`` against each workspace's web client. Pulls
|
||||
public + private channels the bot is a member of, then merges in DMs
|
||||
discovered from session history (IMs aren't useful to enumerate
|
||||
proactively).
|
||||
proactively). If the Slack app lacks channels:read, fall back to session
|
||||
history quietly instead of logging a recurring warning every refresh.
|
||||
"""
|
||||
team_clients = getattr(adapter, "_team_clients", None) or {}
|
||||
if not team_clients:
|
||||
|
|
@ -270,8 +286,15 @@ async def _build_slack(adapter) -> List[Dict[str, Any]]:
|
|||
cursor=cursor,
|
||||
)
|
||||
if not response.get("ok"):
|
||||
detail = f"users.conversations not ok: {response.get('error', 'unknown')}"
|
||||
_warn_slack_directory(team_id, detail)
|
||||
error_code = response.get("error", "unknown")
|
||||
if error_code == "missing_scope":
|
||||
logger.debug(
|
||||
"Channel directory: Slack team %s lacks channels:read; using session history only",
|
||||
team_id,
|
||||
)
|
||||
else:
|
||||
detail = f"users.conversations not ok: {error_code}"
|
||||
_warn_slack_directory(team_id, detail)
|
||||
break
|
||||
for ch in response.get("channels", []):
|
||||
cid = ch.get("id")
|
||||
|
|
@ -288,7 +311,13 @@ async def _build_slack(adapter) -> List[Dict[str, Any]]:
|
|||
if not cursor:
|
||||
break
|
||||
except Exception as e:
|
||||
_warn_slack_directory(team_id, str(e))
|
||||
if _slack_api_error_code(e) == "missing_scope":
|
||||
logger.debug(
|
||||
"Channel directory: Slack team %s lacks channels:read; using session history only",
|
||||
team_id,
|
||||
)
|
||||
else:
|
||||
_warn_slack_directory(team_id, str(e))
|
||||
continue
|
||||
|
||||
# Merge in DM/group entries discovered from session history.
|
||||
|
|
|
|||
|
|
@ -568,21 +568,40 @@ class TestBuildSlack:
|
|||
|
||||
assert {e["id"] for e in entries} == {"C001"}
|
||||
|
||||
def test_response_not_ok_breaks_pagination_for_that_workspace(self, tmp_path):
|
||||
def test_response_not_ok_missing_scope_falls_back_quietly(self, tmp_path, caplog):
|
||||
client = _make_slack_client([
|
||||
{"ok": False, "error": "missing_scope"},
|
||||
])
|
||||
with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}):
|
||||
with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}), caplog.at_level("WARNING"):
|
||||
entries = asyncio.run(_build_slack(_make_slack_adapter({"T1": client})))
|
||||
|
||||
assert entries == []
|
||||
assert "missing_scope" not in caplog.text
|
||||
|
||||
def test_missing_scope_exception_falls_back_quietly(self, tmp_path, caplog):
|
||||
class SlackLikeError(Exception):
|
||||
def __init__(self):
|
||||
super().__init__("The request to the Slack API failed")
|
||||
self.response = {"ok": False, "error": "missing_scope"}
|
||||
|
||||
client = MagicMock()
|
||||
client.users_conversations = AsyncMock(side_effect=SlackLikeError())
|
||||
|
||||
with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}), caplog.at_level("WARNING"):
|
||||
entries = asyncio.run(_build_slack(_make_slack_adapter({"T1": client})))
|
||||
|
||||
assert entries == []
|
||||
assert "missing_scope" not in caplog.text
|
||||
|
||||
def test_repeated_workspace_errors_are_warning_throttled(
|
||||
self, tmp_path, caplog, monkeypatch
|
||||
):
|
||||
# NOTE: uses a non-missing_scope error code — missing_scope is
|
||||
# demoted to DEBUG entirely (quiet channels:read fallback), so the
|
||||
# throttle path only sees other recurring workspace errors.
|
||||
client = _make_slack_client([
|
||||
{"ok": False, "error": "missing_scope"},
|
||||
{"ok": False, "error": "missing_scope"},
|
||||
{"ok": False, "error": "ratelimited"},
|
||||
{"ok": False, "error": "ratelimited"},
|
||||
])
|
||||
_slack_directory_warning_last.clear()
|
||||
monkeypatch.setattr("gateway.channel_directory.time.monotonic", lambda: 1000.0)
|
||||
|
|
@ -597,7 +616,7 @@ class TestBuildSlack:
|
|||
if record.levelname == "WARNING"
|
||||
]
|
||||
assert len(warning_messages) == 1
|
||||
assert "missing_scope" in warning_messages[0]
|
||||
assert "ratelimited" in warning_messages[0]
|
||||
assert any(
|
||||
"suppressed repeated Slack channel list failure" in record.getMessage()
|
||||
for record in caplog.records
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue