fix(tools): honor explicit empty platform toolsets - #107452
Open
BowmanStephen wants to merge 1 commit into
Open
BowmanStephen wants to merge 1 commit into
BowmanStephen wants to merge 1 commit into
Conversation
An explicitly empty `platform_toolsets` entry (e.g. `discord: []`) is
meant to be a deny-all selection, distinct from an omitted key that falls
back to the platform composite. `_get_platform_tools` only honored that
for `context_engine`; it still ran `_recover_platform_native_toolsets`
and `_merge_mcp_servers`, so non-configurable toolsets such as `kanban`
were re-added to a surface the user deliberately disabled.
Return an empty set right after normalising the saved list when it is
explicitly empty, before native-toolset recovery and default MCP
injection. The `context_engine` guard no longer needs the empty-list
special case, since that path has already returned.
The test pins that `{"cli": ["kanban"], "discord": [], "cron": []}`
resolves to `{"kanban"}` for cli and an empty set for discord and cron.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
teknium1
added a commit
that referenced
this pull request
Sep 12, 2026
…he opt-in contract Salvage follow-up to Xipong's #107736. Kept the core: `kanban` is a configurable, default-off toolset whose check_fn answers the schema build's own selection (ContextVar) instead of the legacy top-level `toolsets` key, so `platform_toolsets.<platform>: [.., kanban]` — what `hermes tools enable kanban --platform X` writes — actually reaches the gateway agent's tool schema. Dropped the `tui_gateway/server.py` change: turning an explicitly empty CLI selection from "all" into "nothing" is a separate behaviour flip already tracked by #107452, not part of this bug. The two TUI loader tests that asserted `kanban` is auto-recovered onto a saved `[memory]` list now assert the opposite: a configurable opt-in is never recovered.
5 of 6 tasks
mrkillbob
pushed a commit
to mrkillbob/hermes-agent
that referenced
this pull request
Sep 13, 2026
…he opt-in contract Salvage follow-up to Xipong's NousResearch#107736. Kept the core: `kanban` is a configurable, default-off toolset whose check_fn answers the schema build's own selection (ContextVar) instead of the legacy top-level `toolsets` key, so `platform_toolsets.<platform>: [.., kanban]` — what `hermes tools enable kanban --platform X` writes — actually reaches the gateway agent's tool schema. Dropped the `tui_gateway/server.py` change: turning an explicitly empty CLI selection from "all" into "nothing" is a separate behaviour flip already tracked by NousResearch#107452, not part of this bug. The two TUI loader tests that asserted `kanban` is auto-recovered onto a saved `[memory]` list now assert the opposite: a configurable opt-in is never recovered. (cherry picked from commit 9b9026e)
mrkillbob
pushed a commit
to mrkillbob/hermes-agent
that referenced
this pull request
Sep 13, 2026
…he opt-in contract Salvage follow-up to Xipong's NousResearch#107736. Kept the core: `kanban` is a configurable, default-off toolset whose check_fn answers the schema build's own selection (ContextVar) instead of the legacy top-level `toolsets` key, so `platform_toolsets.<platform>: [.., kanban]` — what `hermes tools enable kanban --platform X` writes — actually reaches the gateway agent's tool schema. Dropped the `tui_gateway/server.py` change: turning an explicitly empty CLI selection from "all" into "nothing" is a separate behaviour flip already tracked by NousResearch#107452, not part of this bug. The two TUI loader tests that asserted `kanban` is auto-recovered onto a saved `[memory]` list now assert the opposite: a configurable opt-in is never recovered. (cherry picked from commit 9b9026e)
mrkillbob
pushed a commit
to mrkillbob/hermes-agent
that referenced
this pull request
Sep 13, 2026
…he opt-in contract Salvage follow-up to Xipong's NousResearch#107736. Kept the core: `kanban` is a configurable, default-off toolset whose check_fn answers the schema build's own selection (ContextVar) instead of the legacy top-level `toolsets` key, so `platform_toolsets.<platform>: [.., kanban]` — what `hermes tools enable kanban --platform X` writes — actually reaches the gateway agent's tool schema. Dropped the `tui_gateway/server.py` change: turning an explicitly empty CLI selection from "all" into "nothing" is a separate behaviour flip already tracked by NousResearch#107452, not part of this bug. The two TUI loader tests that asserted `kanban` is auto-recovered onto a saved `[memory]` list now assert the opposite: a configurable opt-in is never recovered. (cherry picked from commit 9b9026e)
mrkillbob
pushed a commit
to mrkillbob/hermes-agent
that referenced
this pull request
Sep 13, 2026
…he opt-in contract Salvage follow-up to Xipong's NousResearch#107736. Kept the core: `kanban` is a configurable, default-off toolset whose check_fn answers the schema build's own selection (ContextVar) instead of the legacy top-level `toolsets` key, so `platform_toolsets.<platform>: [.., kanban]` — what `hermes tools enable kanban --platform X` writes — actually reaches the gateway agent's tool schema. Dropped the `tui_gateway/server.py` change: turning an explicitly empty CLI selection from "all" into "nothing" is a separate behaviour flip already tracked by NousResearch#107452, not part of this bug. The two TUI loader tests that asserted `kanban` is auto-recovered onto a saved `[memory]` list now assert the opposite: a configurable opt-in is never recovered. (cherry picked from commit 9b9026e)
mrkillbob
pushed a commit
to mrkillbob/hermes-agent
that referenced
this pull request
Sep 13, 2026
…he opt-in contract Salvage follow-up to Xipong's NousResearch#107736. Kept the core: `kanban` is a configurable, default-off toolset whose check_fn answers the schema build's own selection (ContextVar) instead of the legacy top-level `toolsets` key, so `platform_toolsets.<platform>: [.., kanban]` — what `hermes tools enable kanban --platform X` writes — actually reaches the gateway agent's tool schema. Dropped the `tui_gateway/server.py` change: turning an explicitly empty CLI selection from "all" into "nothing" is a separate behaviour flip already tracked by NousResearch#107452, not part of this bug. The two TUI loader tests that asserted `kanban` is auto-recovered onto a saved `[memory]` list now assert the opposite: a configurable opt-in is never recovered. (cherry picked from commit 9b9026e)
mrkillbob
added a commit
to mrkillbob/hermes-agent
that referenced
this pull request
Sep 13, 2026
* fix(kanban): honor explicit platform tool opt-ins across configuration surfaces (cherry picked from commit 3d7f773) * fix(kanban): drop the TUI empty-selection change and refit tests to the opt-in contract Salvage follow-up to Xipong's NousResearch#107736. Kept the core: `kanban` is a configurable, default-off toolset whose check_fn answers the schema build's own selection (ContextVar) instead of the legacy top-level `toolsets` key, so `platform_toolsets.<platform>: [.., kanban]` — what `hermes tools enable kanban --platform X` writes — actually reaches the gateway agent's tool schema. Dropped the `tui_gateway/server.py` change: turning an explicitly empty CLI selection from "all" into "nothing" is a separate behaviour flip already tracked by NousResearch#107452, not part of this bug. The two TUI loader tests that asserted `kanban` is auto-recovered onto a saved `[memory]` list now assert the opposite: a configurable opt-in is never recovered. (cherry picked from commit 9b9026e) * fix(redact): AgentMail prefix rule matches any opaque key body, not only hex Review finding: the salvaged rule assumed a lowercase-hex grammar that AgentMail's docs do not establish (only the `am_` / `am_org_` prefix is documented), so a non-hex key would have gone unmasked. Discriminate on what actually separates keys from identifiers: an alphanumeric body with no `_`/`-` and a 20-char floor. `am_example_identifier_123` still passes. (cherry picked from commit 848075c) * fix(mcp): same-named MCP servers with different credentials connect per profile; owner /reload-mcp keeps adopters' tools Under gateway.multiplex_profiles every connection ledger in tools/mcp_tool.py (_servers, _server_scope_keys/_server_tool_scopes, connecting/error/cooldown maps, the circuit breaker, lazy schema-cache configs, trust metadata) was keyed by the bare server NAME. The common per-tenant layout — each profile names its server `github`/`notion` with its own token — gave only the first profile a connection: the second profile's register_mcp_servers saw the name as "already connected", refused to adopt it (different credentials, 4ddbcbd), and left the profile silently tool-less with a healthy-looking `configured` status (NousResearch#106005 Bug 1/2, NousResearch#91654). Siblings of the same bug: profile A's failing `x` put profile B's healthy `x` into A's 10-minute connect cooldown and A's open circuit breaker short-circuited B's calls; toolsets._resolve_toolset_memo was not scope-keyed, so B resolved A's `mcp-<server>` tool names. Keys are now the connection key from the new tools/mcp_tool_scope.py: the bare name outside a multiplexer (single-profile processes are unchanged) and (owner_scope, name) under one. Call-time lookups (_resolve_server_key) prefer the calling scope's own connection, then a shared connection it adopted, so identical-route profiles still share one subprocess. _select_new_servers, the cooldown/breaker/trust maps, lazy registration and get_mcp_status all read and write through the composite key; teardown resolves a task's key by identity (the MCP loop has no profile context). The toolset memo key includes registry.current_scope_key(). An owner's scoped /reload-mcp tore down its connection and, with it, every adopting profile's tool overlay; nothing re-ran the adopters' discovery until they reloaded. shutdown_mcp_servers(scope=) now records the orphaned adopters and register_mcp_servers re-registers them under their own home + secret scope once the owner's rediscovery pass completes. Docs: multi-profile-gateways.md states the per-profile connection rule. Fixes NousResearch#106005 Fixes NousResearch#91654 Co-authored-by: Bergmann89 <info@bergmann89.de> Co-authored-by: Izzy-Gottz <srulynj@gmail.com> (cherry picked from commit ceaf622) * test(kanban): align worker toolset expectation with explicit opt-in * docs(kanban): per-platform opt-in is the documented path (hermes tools enable kanban --platform X) (cherry picked from commit 819988a) * fix(mcp): preserve scoped reload names and stdio identities * fix(profiles): --clone leaves messaging channels behind; --clone-channels opts in A cloned profile carried the source's TELEGRAM_BOT_TOKEN, DISCORD_BOT_TOKEN, allowlists, WHATSAPP_ENABLED, API_SERVER_KEY and the platforms:/telegram:/ discord: config sections byte-for-byte. Standalone, that made two gateways fight over one bot's long-poll; under multiplex it blocked `hermes gateway migrate --multiplex` with one duplicate-credential finding per platform per clone (18 on a real 10-profile install). Every clone entry point (CLI --clone/--clone-from/--clone-all, dashboard POST /api/profiles, TUI/Desktop profiles.create incl. its mirror_credentials .env copy) now strips channel settings after the copy. The key set is derived from the adapters — Platform enum + plugin registry (required_env, allowed_users_env, allow_all_env, cron_deliver_env_var), the gateway env table (gateway.config_env._ENV_STEPS / _ENV_ENABLE_CREDENTIALS) and each platform's env prefix — so a new adapter is covered without a hand list. --clone-all also drops pairing/WhatsApp-session/gateway ledgers. Provider and tool keys, the model block, memory, skills and SOUL.md are untouched. `--clone-channels` (REST/RPC: clone_channels) keeps them; it is refused when a live multiplexer already serves the source and otherwise warns which platforms are now shared. `hermes profile list` prints the same warning for existing clones whose bot credential is byte-identical to the default's. The dashboard's per-platform env-prefix table moves into profile_channels so Channels-page cards and the clone stripper share one definition. * fix(gateway): register gateway.multiplex_profiles; explicit migrate --multiplex flips it with no standalone secondary `hermes config set gateway.multiplex_profiles true` warned "not a recognized config key" although gateway/config.py reads it: the key (and profile_routes) were never in DEFAULT_CONFIG["gateway"]. Both are registered with their doc comment; the CLI loaders deep-merge new keys, so no _config_version bump. `hermes gateway migrate --multiplex` with two or more profiles but no secondary running its own gateway printed "nothing to migrate" and left the flag OFF. The explicit command now applies the one remaining step — flag on, default gateway (re)started, the same rollback manifest (empty secondaries) for --standalone. `hermes update`'s automatic hook keeps treating that case as a no-op: it never flips modes on an install where nothing was running. * fix(mcp): scope provenance and parallel policy * fix(gateway): preserve upstream multiplex migration dependencies * feat(gateway): multiplexer hot-serves profiles created while it runs, unroutes deleted ones A `gateway.multiplex_profiles` gateway enumerated `profiles/` once at boot, so a profile created afterwards (CLI, dashboard, Desktop, TUI) was never served until `hermes gateway restart`; Desktop and the dashboard gave no reminder, so a new profile's bot simply never connected. The served set is now reconciled at runtime (`gateway/run_profile_reconcile.py`): - `hermes_cli/profiles.py` create/delete ping the multiplexer over its control socket (new `rescan-profiles` verb); a supervised watcher rescans every 30s as the safety net. - A new profile gets its adapters under its own runtime scope from its config/.env (`_start_one_profile_adapters`, same duplicate-credential guard as boot, now seeded with the LIVE secondaries' claims), `served_profiles` in gateway_state.json is updated, MCP discovery + log routing run for it. Other profiles' adapters are never touched. - A served profile whose config.yaml/.env changed is re-scanned so a token added after create builds the adapter; already-live/queued platforms are skipped (no second poller). - A deleted profile (tombstone) has its reconnects cancelled, adapters torn down, pairing/busy bookkeeping and cached agents dropped, and this process's SQLite / memory-store handles released so the deleter's rmtree succeeds. - The in-process cron ticker takes a live enumerator so new profiles' jobs fire. - PUT /api/messaging/platforms/<id>?profile=X returns `hot_served` when a live multiplexer rebuilt X's adapters; Desktop/dashboard skip the restart banner then. - `hermes profile create` confirms hot-serve; the restart reminder stays for a gateway that did not pick the profile up (older build / signal failed). (cherry picked from commit d1dbb0a) * fix(gateway): hot-serve reaches pooled Desktop backends; deleted profiles leave no stale runtime entries - PUT /api/messaging/platforms on a pooled `hermes --profile X serve` arrives without ?profile= (Desktop local topology, NousResearch#109088): resolve the hot-serve target from the process's own profile so the multiplexer is pinged and the UI skips the restart banner. - A profile deleted while the reconcile lock was held by its own adapter connect was recorded back into served_profiles; re-check the live set before recording. - Drop a deleted profile's `<name>:<platform>` runtime-status entries instead of leaving them as `stopped`. (cherry picked from commit 2d121aa) * fix(gateway): a secondary API_SERVER_KEY no longer skips the profile; start/install/status honour the live multiplexer Under gateway.multiplex_profiles the default gateway serves every profile, yet four startup/status paths still reasoned from the wrong source: * A secondary profile's API_SERVER_KEY (which the docs REQUIRE for /p/<profile>/ auth) auto-enabled api_server in that profile's config, so _load_secondary_profile_config raised SecondaryPortBindingConfigError and the whole profile was skipped. gateway/config_env.py::_enable_from_env now leaves `enabled` alone for port-binding platforms while a multiplexer loads a NON-default profile (home override + multiplex flag, the same signal gateway.config uses for scoped reads); the credential still lands in extra so the shared listener can authenticate the prefix. Default profile unchanged. * "Is this profile served?" was re-derived from the default config.yaml plus GATEWAY_MULTIPLEX_PROFILES as seen by the CLI process. `hermes -p coder ...` loads coder's .env, so an env-only opt-in on the default profile was invisible (guard never fired, status said stopped) and an allowlist edit flipped the answer before the restart. named_profile_served_by_running_multiplexer now reads the pid-verified default gateway_state.json served_profiles (written by _record_served_profiles) first and falls back to config derivation only when the key is absent. The record helpers live in hermes_cli/gateway_multiplex_served.py. * The served-profile guard ran only inside `gateway run`. `hermes -p X gateway start|install|restart` reached the service manager, whose unit then exited 78 forever (systemd parks it while the CLI prints "started"; launchd KeepAlive respawns every 30 s). The service verbs now run the same guard up front (exit 78, same message) and accept --force; the Desktop /api/gateway/start route returns 409 for a served profile instead of spawning a doomed child. * Status surfaces disagreed: `hermes -p coder status` said stopped, `hermes -p coder cron status` said "cron jobs will NOT fire" while `cron list` said fine, and the default `hermes status` never listed served profiles. Both now route through the probe / the recorded served set. The -p/--profile matcher in _scan_gateway_pids and gateway.status._command_line_belongs_to_profile compares the flag token for equality (`-p ops` no longer claims -- or lets `gateway stop` SIGTERM -- an `-p ops-2` gateway). Docs: multi-profile-gateways.md now describes the start/install refusal, the --force flags, the API_SERVER_KEY behaviour and the single default-home gateway_state.json (the per-profile runtime_status.json claim was wrong). Fixes NousResearch#100397 Addresses NousResearch#89726 NousResearch#97360 NousResearch#71344 (cherry picked from commit d002c1864a7b6a22c53758b16b7b0cc79aea2edf) (cherry picked from commit 37dcc0a) * fix(gateway): complete profile multiplex migration review fixes * fix(mcp): preserve scoped connection provenance across reloads * fix(desktop): keep hot-served messaging updates type-safe * fix(gateway): restore multiplex CI compatibility seams * fix(gateway): address remaining multiplex review feedback --------- Co-authored-by: Xipong <217837358+Xipong@users.noreply.github.com> Co-authored-by: teknium1 <127238744+teknium1@users.noreply.github.com> Co-authored-by: Mike DeMott <25466867+mrkillbob@users.noreply.github.com>
Author
|
Friendly ping for a first review — still mergeable and untouched since opening. Happy to rebase if it has drifted. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
An explicitly empty
platform_toolsetsentry such asdiscord: []is documented as a deny-all selection, distinct from omitting the key (which falls back to the platform composite)._get_platform_toolsonly honoured that forcontext_engine: it still ran_recover_platform_native_toolsetsand_merge_mcp_servers, so non-configurable toolsets such askanbanwere re-added to surfaces the user had deliberately disabled. On current main,{"cli": ["kanban"], "discord": [], "cron": []}resolves to['kanban']for all three platforms.Change
Return an empty set right after the saved list is normalised when it is explicitly empty, before native-toolset recovery and default-MCP injection. The
context_engineguard drops its now-unreachable empty-list special case.Tests
test_explicit_empty_platform_toolsets_disable_non_configurable_toolsetspins that the config above resolves to{"kanban"}for cli and an empty set for discord and cron.tests/hermes_cli/test_tools_config.pyplus the related toolset / CLI / gateway / cron test files pass (213 passed, 6 skipped).🤖 Generated with Claude Code