Skip to content

[fix] gateway pairing hint for profiles - #37758

Closed
TreyDong wants to merge 1 commit into
NousResearch:mainfrom
TreyDong:codex/profile-aware-pairing-hint
Closed

[fix] gateway pairing hint for profiles#37758
TreyDong wants to merge 1 commit into
NousResearch:mainfrom
TreyDong:codex/profile-aware-pairing-hint

Conversation

@TreyDong

@TreyDong TreyDong commented Jun 3, 2026

Copy link
Copy Markdown

Summary

  • Make unauthorized DM pairing instructions profile-aware.
  • Keep the default profile hint unchanged, while non-default profiles now show the profile alias command, for example invest pairing approve feishu UXWBV4E8.
  • Add a regression test covering a Feishu DM pairing request from a named profile.

Root cause

The gateway pairing response always rendered hermes pairing approve <platform> <code>. Pairing state is stored under the active profile's Hermes home, so approving a code from the default profile can miss pending requests created by a non-default profile gateway.

Validation

  • uv run --python C:\Python313\python.exe python -m pytest tests/gateway/test_unauthorized_dm_behavior.py::test_unauthorized_dm_pairing_hint_includes_active_profile tests/gateway/test_unauthorized_dm_behavior.py::test_unauthorized_dm_pairs_by_default --timeout-method=thread -q
  • git diff --check -- gateway/run.py tests/gateway/test_unauthorized_dm_behavior.py

@TreyDong
TreyDong marked this pull request as ready for review June 3, 2026 01:01
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/gateway Gateway runner, session dispatch, delivery area/auth Authentication, OAuth, credential pools labels Jun 3, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Approved

Overview

Fixes gateway pairing hint to be profile-aware so pairing instructions show the correct profile context.

✅ Looks Good

  • Small, focused change: 44 additions, 1 deletion
  • Fixes a UX issue where pairing shows incorrect profile info
  • Makes the hint respect the active profile
  • No security concerns

Reviewed by Hermes Agent

@TreyDong TreyDong changed the title [codex] fix gateway pairing hint for profiles [fix] gateway pairing hint for profiles Jun 3, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for identifying the profile-scoped pairing workflow.

Problems

  • Current main has a multiplex pairing model that needs more than a text change: the unauthorized-DM branch still uses self.pairing_store for rate limiting and code creation at gateway/run.py:8973-8997, while authorization uses _pairing_store_for(source) in gateway/authz_mixin.py:249-262. A secondary-profile request would therefore create its code in the global store.
  • The proposed helper reads the process-active profile, but multiplex adapters stamp the actual inbound profile on source.profile (gateway/run.py:8481-8485). It would not identify the secondary profile receiving the DM.
  • A profile name is not necessarily an executable alias: wrappers may be custom or absent (hermes_cli/profiles.py:435-475, 539-610).

Suggested changes

  • Route pairing generation, rate limiting, and the hint through the same store/profile selected from source.
  • Use hermes -p <profile> pairing approve ..., or resolve the actual wrapper alias, rather than assuming the profile name is on PATH.
  • Add a multiplex-source regression covering store selection as well as rendered text.

Automated hermes-sweeper review.

Comment thread gateway/run.py
profile_name = "default"

if profile_name in ("default", "custom"):
return f"`hermes pairing approve {platform_name} {code}`"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A profile name is not guaranteed to be a PATH command: wrappers can be custom-named or omitted (hermes_cli/profiles.py:435-475, 539-610). On current main this also needs to derive the target from source.profile, not the process-active profile, for multiplexed secondary adapters.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users area/profiles Multi-profile isolation, HERMES_HOME scoping labels Jul 14, 2026
@OutThisLife

Copy link
Copy Markdown
Collaborator

Right problem — a non-default profile's pairing hint told the owner to run a command that approves into the wrong store. #74446 covers it via #70932's hint, which emits the hermes -p <profile> pairing approve … form off the store that actually issued the code rather than re-deriving the active profile at send time. Thanks for flagging it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools area/profiles Multi-profile isolation, HERMES_HOME scoping comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants