mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-26 17:38:36 +00:00
fix(setup): stop asking about self-configuring platform knobs
Some checks are pending
CI / Detect affected areas (push) Waiting to run
CI / Python tests (push) Blocked by required conditions
CI / Python lints (push) Blocked by required conditions
CI / JS & TS checks (push) Blocked by required conditions
CI / Desktop E2E (push) Blocked by required conditions
CI / Docs Site (push) Blocked by required conditions
CI / Deny unrelated histories (push) Blocked by required conditions
CI / Check contributors (push) Blocked by required conditions
CI / Check uv.lock (push) Blocked by required conditions
CI / package-lock.json diff (push) Blocked by required conditions
CI / Lint Docker scripts (push) Blocked by required conditions
CI / Build&Test Docker image (push) Blocked by required conditions
CI / Supply-chain scan (push) Blocked by required conditions
CI / Review label gate (push) Blocked by required conditions
CI / OSV scan (push) Waiting to run
CI / CI review comment (live) (push) Blocked by required conditions
CI / All required checks pass (push) Blocked by required conditions
CI / CI timing report (push) Blocked by required conditions
Deploy Site / deploy-vercel (push) Waiting to run
Deploy Site / deploy-docs (push) Waiting to run
Docker Build, Test, and Publish / build (amd64, type=gha,scope=docker-amd64, type=gha,mode=max,scope=docker-amd64, linux/amd64, ubuntu-latest) (push) Waiting to run
Docker Build, Test, and Publish / build (arm64, type=gha,scope=docker-arm64, type=gha,mode=max,scope=docker-arm64, linux/arm64, ubuntu-24.04-arm) (push) Waiting to run
Docker Build, Test, and Publish / publish (amd64, type=gha,scope=docker-amd64, type=gha,mode=max,scope=docker-amd64, linux/amd64, ubuntu-latest) (push) Blocked by required conditions
Docker Build, Test, and Publish / publish (arm64, type=gha,scope=docker-arm64, type=gha,mode=max,scope=docker-arm64, linux/arm64, ubuntu-24.04-arm) (push) Blocked by required conditions
Docker Build, Test, and Publish / merge (push) Blocked by required conditions
auto-fix lint issues & formatting / Generate eslint --fix patch (push) Waiting to run
auto-fix lint issues & formatting / Apply patch (push) Blocked by required conditions
Some checks are pending
CI / Detect affected areas (push) Waiting to run
CI / Python tests (push) Blocked by required conditions
CI / Python lints (push) Blocked by required conditions
CI / JS & TS checks (push) Blocked by required conditions
CI / Desktop E2E (push) Blocked by required conditions
CI / Docs Site (push) Blocked by required conditions
CI / Deny unrelated histories (push) Blocked by required conditions
CI / Check contributors (push) Blocked by required conditions
CI / Check uv.lock (push) Blocked by required conditions
CI / package-lock.json diff (push) Blocked by required conditions
CI / Lint Docker scripts (push) Blocked by required conditions
CI / Build&Test Docker image (push) Blocked by required conditions
CI / Supply-chain scan (push) Blocked by required conditions
CI / Review label gate (push) Blocked by required conditions
CI / OSV scan (push) Waiting to run
CI / CI review comment (live) (push) Blocked by required conditions
CI / All required checks pass (push) Blocked by required conditions
CI / CI timing report (push) Blocked by required conditions
Deploy Site / deploy-vercel (push) Waiting to run
Deploy Site / deploy-docs (push) Waiting to run
Docker Build, Test, and Publish / build (amd64, type=gha,scope=docker-amd64, type=gha,mode=max,scope=docker-amd64, linux/amd64, ubuntu-latest) (push) Waiting to run
Docker Build, Test, and Publish / build (arm64, type=gha,scope=docker-arm64, type=gha,mode=max,scope=docker-arm64, linux/arm64, ubuntu-24.04-arm) (push) Waiting to run
Docker Build, Test, and Publish / publish (amd64, type=gha,scope=docker-amd64, type=gha,mode=max,scope=docker-amd64, linux/amd64, ubuntu-latest) (push) Blocked by required conditions
Docker Build, Test, and Publish / publish (arm64, type=gha,scope=docker-arm64, type=gha,mode=max,scope=docker-arm64, linux/arm64, ubuntu-24.04-arm) (push) Blocked by required conditions
Docker Build, Test, and Publish / merge (push) Blocked by required conditions
auto-fix lint issues & formatting / Generate eslint --fix patch (push) Waiting to run
auto-fix lint issues & formatting / Apply patch (push) Blocked by required conditions
Connecting Discord asked five questions when the platform needs one. The card listed a home channel ID you need Developer Mode to copy, an allow-all-users security toggle, a reply-threading preference, and a home channel display name — all with working defaults, none discoverable from the form. Drops them from the setup surfaces entirely: the dashboard/Desktop channel cards and the `hermes setup gateway` wizard. Discord is now bot token + allowlist. Matrix drops from 11 fields to 7, Mattermost from 6 to 3. Suffix-matched (`*_HOME_CHANNEL*`, `*_ALLOW_ALL_USERS`, `*_REPLY_TO_MODE`, `*_REQUIRE_MENTION`, `*_AUTO_THREAD`, `*_FREE_RESPONSE_*`, `*_PROXY`) so plugin platforms nobody enumerated get the same treatment. Allowlists deliberately stay — the gateway denies everyone until one is set, so that IS the decision a new user has to make. Required credentials are never hidden. Nothing is removed from the product. The vars still work through `hermes config set`, .env, and config.yaml, the gateway reads them unchanged, and dropping them from the cards hands them back to the Keys page rather than orphaning them (Keys hides only what a Channels card owns).
This commit is contained in:
parent
e289e561c7
commit
339d968689
5 changed files with 270 additions and 9 deletions
|
|
@ -5470,6 +5470,9 @@ def _set_platform_unauthorized_dm_behavior(platform_key: str, behavior: str) ->
|
|||
|
||||
def _setup_standard_platform(platform: dict):
|
||||
"""Interactive setup for Telegram, Discord, or Slack."""
|
||||
# Same hidden-knob list the dashboard/Desktop channel cards use.
|
||||
from hermes_cli.setup_hidden_env import is_setup_hidden_env as _is_setup_hidden_env
|
||||
|
||||
emoji = platform["emoji"]
|
||||
label = platform["label"]
|
||||
token_var = platform["token_var"]
|
||||
|
|
@ -5522,7 +5525,22 @@ def _setup_standard_platform(platform: dict):
|
|||
|
||||
allowed_val_set = None # Track if user set an allowlist (for home channel offer)
|
||||
|
||||
for var in platform["vars"]:
|
||||
# Skip the knobs the setup forms hide (home channel, reply mode, proxy,
|
||||
# mention behavior). They're self-configuring or already correct by
|
||||
# default — /sethome sets the home channel on the first chat — so asking
|
||||
# about each one turned a 2-question setup into a 5-question one. Same
|
||||
# exclusion the dashboard/Desktop cards use, so the surfaces agree.
|
||||
# Required credentials are never skipped.
|
||||
required_names = {token_var}
|
||||
setup_vars = [
|
||||
v
|
||||
for v in platform["vars"]
|
||||
if v["name"] in required_names
|
||||
or v.get("is_allowlist")
|
||||
or not _is_setup_hidden_env(v["name"])
|
||||
]
|
||||
|
||||
for var in setup_vars:
|
||||
print()
|
||||
print_info(f" {var['help']}")
|
||||
existing = get_env_value(var["name"])
|
||||
|
|
|
|||
56
hermes_cli/setup_hidden_env.py
Normal file
56
hermes_cli/setup_hidden_env.py
Normal file
|
|
@ -0,0 +1,56 @@
|
|||
"""Which platform env vars the setup surfaces hide.
|
||||
|
||||
Every messaging platform ships the same handful of knobs that are either set
|
||||
for the user later or already correct by default. Listing them on a setup form
|
||||
turns "paste your bot token" into a five-field interrogation where none of the
|
||||
answers are discoverable — the Discord card asked for a home channel ID you
|
||||
need Developer Mode to copy, next to a reply-threading preference.
|
||||
|
||||
Hiding them is a *presentation* decision only. The env vars keep working
|
||||
through ``hermes config set``, ``.env``, and ``config.yaml``; the gateway reads
|
||||
them exactly as before. This module just says what a new user is asked during
|
||||
setup.
|
||||
|
||||
Lives here rather than in ``web_server`` so the CLI wizard can share it without
|
||||
importing the dashboard's FastAPI surface.
|
||||
"""
|
||||
|
||||
# Suffix match, so plugin adapters nobody enumerated (IRC, SimpleX, LINE, ntfy)
|
||||
# get the same treatment without a code change here.
|
||||
#
|
||||
# *_HOME_CHANNEL* the bot offers /sethome on the first chat
|
||||
# *_ALLOW_ALL_USERS defaults off; enabling it is a security decision
|
||||
# *_REPLY_TO_MODE cosmetic threading preference
|
||||
# *_REPLY_MODE same, Mattermost's spelling
|
||||
# *_REQUIRE_MENTION behavior toggle with a sane default
|
||||
# *_AUTO_THREAD same
|
||||
# *_FREE_RESPONSE_* per-channel tuning, done once the bot is in a server
|
||||
# *_ALLOWED_CHANNELS same
|
||||
# *_PROXY only for networks that block the platform
|
||||
#
|
||||
# Allowlists (*_ALLOWED_USERS) deliberately stay visible: that IS the decision
|
||||
# a new user has to make, and the gateway denies everyone until it's set.
|
||||
SETUP_HIDDEN_ENV_SUFFIXES = (
|
||||
"_HOME_CHANNEL",
|
||||
"_HOME_CHANNEL_NAME",
|
||||
"_HOME_CHANNEL_THREAD_ID",
|
||||
"_HOME_ADDRESS",
|
||||
"_ALLOW_ALL_USERS",
|
||||
"_REPLY_TO_MODE",
|
||||
"_REPLY_MODE",
|
||||
"_REQUIRE_MENTION",
|
||||
"_AUTO_THREAD",
|
||||
"_FREE_RESPONSE_CHANNELS",
|
||||
"_FREE_RESPONSE_ROOMS",
|
||||
"_ALLOWED_CHANNELS",
|
||||
"_PROXY",
|
||||
)
|
||||
|
||||
|
||||
def is_setup_hidden_env(name: str) -> bool:
|
||||
"""True when a var is self-configuring and shouldn't appear in setup forms.
|
||||
|
||||
Callers must still keep any var a platform lists as *required* — hiding a
|
||||
required credential would make that platform unconfigurable from the UI.
|
||||
"""
|
||||
return name.endswith(SETUP_HIDDEN_ENV_SUFFIXES)
|
||||
|
|
@ -8046,7 +8046,6 @@ _PLATFORM_OVERRIDES: dict[str, dict[str, Any]] = {
|
|||
"env_vars": (
|
||||
"DISCORD_BOT_TOKEN",
|
||||
"DISCORD_ALLOWED_USERS",
|
||||
"DISCORD_REPLY_TO_MODE",
|
||||
),
|
||||
"required_env": ("DISCORD_BOT_TOKEN",),
|
||||
},
|
||||
|
|
@ -8485,6 +8484,14 @@ def _platform_env_prefixes(platform_id: str) -> tuple[str, ...]:
|
|||
return (platform_id.upper().replace("-", "_") + "_",)
|
||||
|
||||
|
||||
# Which per-platform knobs the setup UI hides, and why: see
|
||||
# hermes_cli/setup_hidden_env.py. Shared with the `hermes setup gateway`
|
||||
# wizard so the surfaces ask for the same things.
|
||||
from hermes_cli.setup_hidden_env import ( # noqa: E402
|
||||
is_setup_hidden_env as _is_setup_hidden_env,
|
||||
)
|
||||
|
||||
|
||||
def _discover_platform_env_vars(platform_id: str) -> tuple[str, ...]:
|
||||
"""All messaging-category env vars for a platform (override + plugin + prefix)."""
|
||||
prefixes = _platform_env_prefixes(platform_id)
|
||||
|
|
@ -8494,6 +8501,8 @@ def _discover_platform_env_vars(platform_id: str) -> tuple[str, ...]:
|
|||
continue
|
||||
if name in _MESSAGING_KEYS_PAGE_KEYS:
|
||||
continue
|
||||
if _is_setup_hidden_env(name):
|
||||
continue
|
||||
if not any(name.startswith(prefix) for prefix in prefixes):
|
||||
continue
|
||||
keys.append(name)
|
||||
|
|
@ -8505,10 +8514,18 @@ def _merge_platform_env_vars(
|
|||
override: dict[str, Any],
|
||||
plugin_entry: Any | None,
|
||||
) -> tuple[str, ...]:
|
||||
"""Canonical env-var list for a messaging platform card."""
|
||||
"""Canonical env-var list for a messaging platform card.
|
||||
|
||||
Required credentials always survive: a platform that genuinely needs one of
|
||||
the hidden-suffix vars to connect keeps it, since hiding a required field
|
||||
would make the platform unconfigurable.
|
||||
"""
|
||||
discovered = _discover_platform_env_vars(platform_id)
|
||||
if "env_vars" in override:
|
||||
return tuple(dict.fromkeys((*override["env_vars"], *discovered)))
|
||||
explicit = tuple(
|
||||
key for key in override["env_vars"] if not _is_setup_hidden_env(key)
|
||||
)
|
||||
return tuple(dict.fromkeys((*explicit, *discovered)))
|
||||
if plugin_entry is not None and plugin_entry.required_env:
|
||||
return tuple(dict.fromkeys((*tuple(plugin_entry.required_env), *discovered)))
|
||||
return discovered
|
||||
|
|
|
|||
163
tests/hermes_cli/test_setup_hidden_env.py
Normal file
163
tests/hermes_cli/test_setup_hidden_env.py
Normal file
|
|
@ -0,0 +1,163 @@
|
|||
"""Setup surfaces ask only for what a platform can't start without.
|
||||
|
||||
The knobs in SETUP_HIDDEN_ENV_SUFFIXES are self-configuring (/sethome) or
|
||||
already correct by default, so they don't belong on a setup form. They must
|
||||
still be reachable everywhere else — this is a presentation change, not a
|
||||
feature removal.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
|
||||
from hermes_cli.setup_hidden_env import is_setup_hidden_env
|
||||
|
||||
|
||||
class TestIsSetupHiddenEnv:
|
||||
@pytest.mark.parametrize(
|
||||
"key",
|
||||
[
|
||||
"DISCORD_HOME_CHANNEL",
|
||||
"DISCORD_HOME_CHANNEL_NAME",
|
||||
"DISCORD_ALLOW_ALL_USERS",
|
||||
"DISCORD_REPLY_TO_MODE",
|
||||
"MATTERMOST_REPLY_MODE",
|
||||
"TELEGRAM_PROXY",
|
||||
],
|
||||
)
|
||||
def test_self_configuring_knobs_are_hidden(self, key):
|
||||
assert is_setup_hidden_env(key)
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"key",
|
||||
[
|
||||
"DISCORD_BOT_TOKEN",
|
||||
"TELEGRAM_BOT_TOKEN",
|
||||
"SLACK_BOT_TOKEN",
|
||||
"MATTERMOST_URL",
|
||||
# Allowlists stay: the gateway denies everyone until one is set,
|
||||
# so this is the decision a new user actually has to make.
|
||||
"DISCORD_ALLOWED_USERS",
|
||||
"SLACK_ALLOWED_USERS",
|
||||
],
|
||||
)
|
||||
def test_credentials_and_allowlists_stay_visible(self, key):
|
||||
assert not is_setup_hidden_env(key)
|
||||
|
||||
def test_applies_to_plugin_platforms_nobody_enumerated(self):
|
||||
"""Suffix matching is the point — IRC/SimpleX/ntfy get this for free."""
|
||||
for key in (
|
||||
"IRC_ALLOW_ALL_USERS",
|
||||
"SIMPLEX_HOME_CHANNEL",
|
||||
"NTFY_HOME_CHANNEL_NAME",
|
||||
"LINE_ALLOW_ALL_USERS",
|
||||
):
|
||||
assert is_setup_hidden_env(key), key
|
||||
|
||||
|
||||
class TestChannelCards:
|
||||
def test_discord_card_asks_for_token_and_allowlist_only(self):
|
||||
"""The reported bug: the Discord card asked five questions for a
|
||||
one-credential platform."""
|
||||
from hermes_cli.web_server import _build_catalog_entry
|
||||
|
||||
assert set(_build_catalog_entry("discord")["env_vars"]) == {
|
||||
"DISCORD_BOT_TOKEN",
|
||||
"DISCORD_ALLOWED_USERS",
|
||||
}
|
||||
|
||||
def test_no_card_shows_a_hidden_knob(self):
|
||||
from hermes_cli.web_server import _messaging_platform_catalog
|
||||
|
||||
for entry in _messaging_platform_catalog():
|
||||
for key in entry["env_vars"]:
|
||||
assert not is_setup_hidden_env(key), f"{entry['id']}: {key}"
|
||||
|
||||
def test_required_credentials_are_never_hidden(self):
|
||||
"""Hiding a required var would make its platform unconfigurable."""
|
||||
from hermes_cli.web_server import _messaging_platform_catalog
|
||||
|
||||
for entry in _messaging_platform_catalog():
|
||||
for key in entry["required_env"]:
|
||||
assert key in entry["env_vars"], f"{entry['id']}: {key}"
|
||||
|
||||
def test_hidden_knobs_move_to_the_keys_page_not_into_a_void(self):
|
||||
"""Keys hides what a Channels card owns. Dropping these from the card
|
||||
must hand them back to Keys, not orphan them from every surface."""
|
||||
from hermes_cli.web_server import _channel_managed_env_keys
|
||||
|
||||
managed = _channel_managed_env_keys()
|
||||
for key in (
|
||||
"DISCORD_HOME_CHANNEL",
|
||||
"DISCORD_ALLOW_ALL_USERS",
|
||||
"DISCORD_REPLY_TO_MODE",
|
||||
):
|
||||
assert key not in managed, f"{key} is hidden from both surfaces"
|
||||
|
||||
|
||||
class TestCliWizard:
|
||||
"""`hermes setup gateway` drops the same knobs. Real wizard, scripted
|
||||
stdin, real .env writes under a temp HERMES_HOME."""
|
||||
|
||||
@pytest.fixture
|
||||
def home(self, tmp_path, monkeypatch):
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
return tmp_path
|
||||
|
||||
def _run(self, answers, monkeypatch):
|
||||
import io
|
||||
|
||||
from hermes_cli import gateway as gw
|
||||
|
||||
platform = next(p for p in gw._PLATFORMS if p["key"] == "mattermost")
|
||||
monkeypatch.setattr("sys.stdin", io.StringIO("\n".join(answers) + "\n"))
|
||||
gw._setup_standard_platform(dict(platform))
|
||||
|
||||
def _env(self, home):
|
||||
path = home / ".env"
|
||||
if not path.exists():
|
||||
return {}
|
||||
out = {}
|
||||
for line in path.read_text().splitlines():
|
||||
line = line.strip()
|
||||
if line and not line.startswith("#") and "=" in line:
|
||||
k, v = line.split("=", 1)
|
||||
out[k] = v
|
||||
return out
|
||||
|
||||
def test_wizard_stops_asking_about_hidden_knobs(self, home, monkeypatch, capsys):
|
||||
# Only the three real answers. If the wizard still prompted for the
|
||||
# home channel it would read past them.
|
||||
self._run(
|
||||
["https://mm.example.com", "tok-abc", "user26charid"], monkeypatch
|
||||
)
|
||||
|
||||
written = self._env(home)
|
||||
assert written.get("MATTERMOST_URL") == "https://mm.example.com"
|
||||
assert written.get("MATTERMOST_TOKEN") == "tok-abc"
|
||||
assert "MATTERMOST_HOME_CHANNEL" not in written
|
||||
assert "MATTERMOST_REPLY_MODE" not in written
|
||||
|
||||
out = capsys.readouterr().out
|
||||
assert "Home channel" not in out
|
||||
assert "Reply mode" not in out
|
||||
|
||||
def test_required_token_still_gates_setup(self, home, monkeypatch):
|
||||
"""Proves the token wasn't swept out along with the knobs."""
|
||||
self._run(["https://mm.example.com", ""], monkeypatch)
|
||||
|
||||
assert "MATTERMOST_TOKEN" not in self._env(home)
|
||||
|
||||
|
||||
class TestStillConfigurable:
|
||||
def test_gateway_still_honors_the_env_vars(self, tmp_path, monkeypatch):
|
||||
"""Nothing was removed from the product — only from the setup form."""
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
monkeypatch.setenv("DISCORD_BOT_TOKEN", "t")
|
||||
monkeypatch.setenv("DISCORD_HOME_CHANNEL", "999")
|
||||
monkeypatch.setenv("DISCORD_REPLY_TO_MODE", "off")
|
||||
|
||||
from gateway.config import Platform, load_gateway_config
|
||||
|
||||
discord = load_gateway_config().platforms[Platform.DISCORD]
|
||||
assert discord.home_channel is not None
|
||||
assert discord.home_channel.chat_id == "999"
|
||||
assert discord.reply_to_mode == "off"
|
||||
|
|
@ -3153,6 +3153,14 @@ class TestWebServerEndpoints:
|
|||
assert data["AWS_PROFILE"]["provider"] == "bedrock"
|
||||
|
||||
def test_platform_scoped_messaging_env_vars_are_channel_managed(self):
|
||||
"""Platform-scoped vars belong to a Channels card; cross-cutting
|
||||
gateway vars belong to the Keys page.
|
||||
|
||||
Uses credentials as the example: the self-configuring knobs
|
||||
(*_HOME_CHANNEL, *_ALLOW_ALL_USERS, …) were deliberately dropped from
|
||||
the setup cards and handed back to Keys — see
|
||||
tests/hermes_cli/test_setup_hidden_env.py.
|
||||
"""
|
||||
from hermes_cli.web_server import (
|
||||
_MESSAGING_KEYS_PAGE_KEYS,
|
||||
_build_catalog_entry,
|
||||
|
|
@ -3160,13 +3168,12 @@ class TestWebServerEndpoints:
|
|||
)
|
||||
|
||||
discord = _build_catalog_entry("discord")
|
||||
assert "DISCORD_HOME_CHANNEL" in discord["env_vars"]
|
||||
assert "DISCORD_ALLOW_ALL_USERS" in discord["env_vars"]
|
||||
assert "DISCORD_BOT_TOKEN" in discord["env_vars"]
|
||||
assert "DISCORD_ALLOWED_USERS" in discord["env_vars"]
|
||||
|
||||
managed = _channel_managed_env_keys()
|
||||
assert "DISCORD_HOME_CHANNEL" in managed
|
||||
assert "BLUEBUBBLES_ALLOW_ALL_USERS" in managed
|
||||
assert "MATTERMOST_ALLOW_ALL_USERS" in managed
|
||||
assert "DISCORD_BOT_TOKEN" in managed
|
||||
assert "MATTERMOST_TOKEN" in managed
|
||||
assert "GATEWAY_PROXY_URL" not in managed
|
||||
assert "GATEWAY_PROXY_URL" in _MESSAGING_KEYS_PAGE_KEYS
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue