Skip to content

fix(gateway): skip token platforms with no credential on secondary profiles - #72313

Closed
manny3 wants to merge 3 commits into
NousResearch:mainfrom
manny3:fix/secondary-profile-missing-bot-credential
Closed

manny3 wants to merge 3 commits into
NousResearch:mainfrom
manny3:fix/secondary-profile-missing-bot-credential

Conversation

@manny3

@manny3 manny3 commented Jul 26, 2026

Copy link
Copy Markdown

What does this PR do?

Fixes the mirror case of #64674, which was left open.

#64674 / #65525 fixed the default profile connecting a token platform whose bot credential lived only in a secondary profile's .env, and taught the reconnect watcher to drop such queued configs. The opposite direction still fails: a secondary profile can have discord/telegram enabled — the plugin registry auto-enables them for any profile that lists those plugins — while the credential lives only in the default profile's .env.

That is the normal shape for a profile used purely as a gateway.profile_routes target. Per gateway/profile_routing.py, such a profile supplies the model/tools/memory/persona, and inbound arrives on the default profile's single connection — it is not a second bot and has no reason to hold a token. But _start_one_profile_adapters builds an adapter for it anyway, so every gateway start logs:

ERROR   [Discord] No bot token configured
WARNING ✗ discord failed to connect (profile: mentor)
ERROR   [Telegram] No bot token configured
WARNING ✗ telegram failed to connect (profile: mentor)

for connections that can never succeed. _platform_has_bot_credential() already exists for exactly this decision and is applied at the two other sites that need it (run.py:7758 primary startup, run.py:8740 reconnect watcher). This adds the missing third call site so all three agree.

Reusing the helper rather than writing a fresh check keeps its established semantics: platforms outside PLATFORM_TOKEN_ENV_NAMES are never skipped (Signal session paths, port-binding HTTP adapters), and api_key counts as a credential for adapters that take it as primary.

Interaction with #68746 — please read before merging

#68746 is open and modifies _platform_has_bot_credential() itself, adding a live raw-os.environ fallback for the reconnect path. That fallback is correct there but wrong at this new call site: the process env holds the default profile's tokens, and _profile_runtime_scope deliberately does not mutate os.environ (see its docstring). So if #68746 lands as written, this guard would see the default profile's DISCORD_BOT_TOKEN, decide the routes-only profile "has" a credential, and start connecting the default profile's bot on every secondary profile again — silently reintroducing this bug.

Whichever of the two merges second should make the env re-check resolve through the profile's secret scope (get_secret) rather than raw os.environ, or skip the fallback for the secondary-profile call. There is a code comment at the call site flagging this so it is not lost. Happy to rebase onto #68746 and do that reconciliation here if you'd prefer it in one PR.

Related Issue

Mirror case of #64674 (fixed for the primary path by #65525). No separate issue filed — say the word and I'll open one.

Fixes #

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • gateway/run.py — in _start_one_profile_adapters(), skip a platform when _platform_has_bot_credential() reports no credential for that profile's config, logging at INFO which profile was skipped and why instead of failing an impossible connection. Placed after the existing Platform.RELAY skip.
  • tests/gateway/test_secondary_profile_missing_token_skip.py — new suite (4 cases): missing token (both None and "") is skipped; a profile with its own token still connects; api_key alone counts as a credential; a non-token platform (SIGNAL) is never skipped.

The tests drive the real GatewayRunner._start_one_profile_adapters rather than re-implementing its loop body, so the guard cannot regress silently. The runner stub is deliberately complete enough that removing the guard fails on the meaningful created == [] assertion — my first version died early on an incidental AttributeError instead, which would have made the suite near-worthless as a regression guard.

How to Test

Reproduce (this is the live configuration the bug was found on):

  1. A default profile with DISCORD_BOT_TOKEN in ~/.hermes/.env, and gateway.multiplex_profiles: true with a profile_routes entry pointing a channel at a secondary profile:
    gateway:
      multiplex_profiles: true
      profile_routes:
        - name: mentor-council
          platform: discord
          guild_id: "<guild>"
          chat_id: "<channel>"
          profile: mentor
  2. profiles/mentor/.env with no messaging tokens (correct: one bot token cannot be polled twice, and the same-token detector at run.py:9592 already refuses that).
  3. Start the gateway → on main every start logs No bot token configured + ✗ discord failed to connect (profile: mentor) for both discord and telegram.

Verify the fix — with this branch, the same start instead logs:

INFO Skipping discord on profile 'mentor': no bot credential in this profile's secrets.
     Inbound for this profile arrives on the default profile's connection via gateway.profile_routes.
INFO Skipping telegram on profile 'mentor': ...
INFO Gateway running with 2 platform(s)

Confirmed live on the setup above: zero failed to connect lines, both default-profile platforms still connect, and routing is unaffected — match_profile_route() still resolves the routed channel to mentor and other channels to the default profile.

Automated:

pytest tests/gateway/test_secondary_profile_missing_token_skip.py -q          # 4 passed
pytest tests/gateway/test_64674_multiplex_primary_token_scope.py -q           # 8 passed (sibling suite)
pytest tests/gateway/ -q -k "multiplex or profile"                            # 245 passed, 2 skipped

Checklist

Code

On the full-suite checkbox, left unchecked deliberately. pytest tests/gateway/ -q gives 9927 passed / 14 failed on my machine. I verified none of the 14 are mine: re-running exactly those 14 with and without this patch gives an identical 5 failed / 9 passed both ways. The 9 pass in isolation and only fail in the full run (pre-existing ordering dependence); the remaining 5 are environmental on macOS (test_systemd_notify needs systemd, plus test_shutdown_forensics subprocess spawn, test_background_command media routing, test_busy_session_ack). Happy to paste the full output.

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A — N/A: no user-facing config or API surface changes; the reasoning lives in a code comment at the call site
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A — N/A: no new config keys
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A — N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A — pure control-flow guard on an existing pure-Python predicate; no platform-specific paths
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A — N/A

Screenshots / Logs

Before (on main, every gateway start):

2026-07-26 23:27:24 INFO  gateway.run: ✓ discord connected
2026-07-26 23:27:24 ERROR [Discord] No bot token configured
2026-07-26 23:27:24 WARN  gateway.run: ✗ discord failed to connect (profile: mentor)
2026-07-26 23:27:24 ERROR [Telegram] No bot token configured
2026-07-26 23:27:24 WARN  gateway.run: ✗ telegram failed to connect (profile: mentor)
2026-07-26 23:27:24 INFO  gateway.run: Gateway running with 2 platform(s)

After (this branch, same config, nothing else changed):

2026-07-27 00:06:36 INFO gateway.run: ✓ discord connected
2026-07-27 00:06:36 INFO gateway.run: Skipping discord on profile 'mentor': no bot credential in this
                                     profile's secrets. Inbound for this profile arrives on the default
                                     profile's connection via gateway.profile_routes.
2026-07-27 00:06:36 INFO gateway.run: Skipping telegram on profile 'mentor': ...
2026-07-27 00:06:36 INFO gateway.run: Gateway running with 2 platform(s)

🤖 Generated with Claude Code

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/discord Discord bot adapter platform/telegram Telegram bot adapter area/config Config system, migrations, profiles area/profiles Multi-profile isolation, HERMES_HOME scoping needs-decision Awaiting maintainer decision before any implementation sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Jul 27, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Related to #68746: this secondary-profile call site must decide from the profile-scoped config, while #68746 changes the shared helper to fall back to process environment. Please resolve that contract before merging both.

manny3 and others added 2 commits July 27, 2026 11:49
…ofiles

NousResearch#64674 fixed the DEFAULT profile connecting a token platform whose bot
credential lived only in a secondary profile's .env, and wired the reconnect
watcher to drop such queued configs. The mirror case was left open: a
SECONDARY profile can have discord/telegram enabled — the plugin registry
auto-enables them for any profile listing those plugins — while the credential
lives only in the default profile's .env.

That is the normal shape for a profile used purely as a gateway.profile_routes
target: it supplies the model/tools/memory/persona, and inbound arrives on the
default profile's single connection. _start_one_profile_adapters built an
adapter anyway, so every gateway start logged a bare "No bot token configured"
followed by "x <platform> failed to connect (profile: <name>)" for a connection
that could never succeed.

Apply the existing _platform_has_bot_credential guard in that loop too, so all
three call sites agree. Reusing the helper also keeps the established
semantics: platforms outside PLATFORM_TOKEN_ENV_NAMES are never skipped (Signal
session paths, port-binding HTTP adapters), and api_key counts as a credential
for adapters that take it as primary.

The new suite drives the real _start_one_profile_adapters rather than
re-implementing its loop, with a stub complete enough that a regression fails
on the "no adapter may be built" assertion instead of dying early on a missing
attribute.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ial helper

The guard added in the previous commit reused _platform_has_bot_credential.
That helper is shared with two call sites that act on the DEFAULT profile's
behalf — the multiplex startup skip and the reconnect retry queue — and NousResearch#68746
proposes giving it an os.environ fallback, plus an in-place backfill of the
recovered token onto platform_config.

Both are correct for those call sites: there, os.environ holds the default
profile's own secrets. Both are wrong here. This loop runs outside
_profile_runtime_scope, which deliberately does not mutate os.environ, so a
token visible in the process env belongs to the DEFAULT profile. Accepting it
would build an adapter for a routes-only profile using the default profile's
bot identity — two gateway sessions on one token, inbound landing on whichever
won the race — and the backfill would leave that borrowed token on a config
object we keep in _profile_adapters. That is a quieter, worse version of the
bug this branch exists to fix: the old failure at least announced itself as
"✗ <platform> failed to connect".

The previous commit encoded that contract as a comment asking a future editor
of the shared helper to preserve it. Split out _profile_config_has_bot_credential
instead, so this call site cannot be broken from a distance no matter how the
shared helper evolves. The two keep identical semantics today — platforms
outside PLATFORM_TOKEN_ENV_NAMES are never skipped, api_key counts as a
credential — so this commit is behavior-neutral on its own.

The ~8 duplicated lines are deliberate, against "extend, don't duplicate". The
zero-duplication alternative is to reimplement the shared helper as a delegation
to this primitive, which conflicts with NousResearch#68746 in exactly the hunk it edits and
forces the two PRs to land together. Keeping them textually independent is worth
the duplication; the docstring records why they must not be re-merged.

test_default_profile_token_in_process_env_is_not_borrowed drives the real
_start_one_profile_adapters with the platform's token env var set, and asserts
both that no adapter is built and that platform_config.token is left alone.
Verified it fails if the call site is pointed back at the shared helper with
NousResearch#68746 applied.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@manny3
manny3 force-pushed the fix/secondary-profile-missing-bot-credential branch from 4c563e4 to 01eebe7 Compare July 28, 2026 09:17
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the focused multiplexing fix. The premise still holds on current main: _start_one_profile_adapters loads the secondary profile under its secret scope (gateway/run.py:12520-12521) but then creates every enabled adapter (gateway/run.py:12550-12563) and logs a failed connection when it cannot connect (gateway/run.py:12624-12638).

The proposed config-only predicate fits the existing isolation contract: _profile_runtime_scope installs a per-profile secret mapping without modifying os.environ (gateway/run.py:1778-1809), and PLATFORM_TOKEN_ENV_NAMES explicitly distinguishes token platforms from session- and listener-authenticated platforms (gateway/config.py:577-590). The added tests cover absent/blank credentials, own credentials, API-key credentials, non-token platforms, and process-environment isolation.

GitHub currently reports the PR as mergeable, despite it being behind current main.

Automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 30, 2026
Resolves one conflict in gateway/run.py. Git aligned this branch's
secondary-profile guard against the NousResearch#64674 primary guard in the DEFAULT
profile's startup loop — the two loops are near-identical in shape, so the
textual match landed in the wrong function. Taking either side verbatim would
have been wrong: the primary loop has no profile_name/profile_home in scope.

Resolution keeps main's primary guard where it belongs and re-places this
branch's guard in _start_one_profile_adapters, after the Platform.RELAY skip,
as originally authored. _profile_config_has_bot_credential merged cleanly and
is unchanged.

Verified _platform_has_bot_credential on current main is still config-only —
NousResearch#68746 has not landed, so the contract this branch protects is intact.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
teknium1 pushed a commit that referenced this pull request Sep 2, 2026
…figured profiles (#84079)

Secondary profile startup and reconnect now call the existing
`_platform_has_bot_credential` gate (the same one the primary loop and
primary reconnect use since #64674), so an enabled-in-YAML platform whose
credential is absent from that profile's secret scope is skipped instead
of built with an empty token and fanned out.

Independently reported and fixed in #72313 (@manny3), which added a
duplicate helper; the shared main helper is used here instead.

Co-authored-by: manny3 <16465310+manny3@users.noreply.github.com>
teknium1 pushed a commit that referenced this pull request Sep 2, 2026
…figured profiles (#84079)

Secondary profile startup and reconnect now call the existing
`_platform_has_bot_credential` gate (the same one the primary loop and
primary reconnect use since #64674), so an enabled-in-YAML platform whose
credential is absent from that profile's secret scope is skipped instead
of built with an empty token and fanned out.

Independently reported and fixed in #72313 (@manny3), which added a
duplicate helper; the shared main helper is used here instead.

Co-authored-by: manny3 <16465310+manny3@users.noreply.github.com>
teknium1 pushed a commit that referenced this pull request Sep 2, 2026
…figured profiles (#84079)

Secondary profile startup and reconnect now call the existing
`_platform_has_bot_credential` gate (the same one the primary loop and
primary reconnect use since #64674), so an enabled-in-YAML platform whose
credential is absent from that profile's secret scope is skipped instead
of built with an empty token and fanned out.

Independently reported and fixed in #72313 (@manny3), which added a
duplicate helper; the shared main helper is used here instead.

Co-authored-by: manny3 <16465310+manny3@users.noreply.github.com>
@teknium1

teknium1 commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Thanks for this PR. Merged via #101244 (0fd9218) on current main — routed multiplex profiles stop leaking .env into os.environ or borrowing default creds.

#101244 won as the consolidated fix because it covers the whole multiplex-profile bug class in one change (with tests) rather than the single symptom addressed here; this PR is superseded by it.
You are credited via Co-authored-by / in the PR body of #101244 as noted there.

If anything from your original change is still missing on main >= 0fd9218, please open a fresh PR/issue against main and tag it. Thanks again.

@teknium1 teknium1 closed this Sep 2, 2026
melon-xf added a commit to melon-xf/hermes-agent that referenced this pull request Sep 3, 2026
…figured profiles (NousResearch#84079)

Secondary profile startup and reconnect now call the existing
`_platform_has_bot_credential` gate (the same one the primary loop and
primary reconnect use since NousResearch#64674), so an enabled-in-YAML platform whose
credential is absent from that profile's secret scope is skipped instead
of built with an empty token and fanned out.

Independently reported and fixed in NousResearch#72313 (@manny3), which added a
duplicate helper; the shared main helper is used here instead.

Co-authored-by: manny3 <16465310+manny3@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles area/profiles Multi-profile isolation, HERMES_HOME scoping comp/gateway Gateway runner, session dispatch, delivery needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists platform/discord Discord bot adapter platform/telegram Telegram bot adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants