Skip to content

feat(gateway): attribute every inbound sender with stable identity - #76516

Open
NicholaiVogel wants to merge 1 commit into
NousResearch:mainfrom
NicholaiVogel:fix/gateway-dm-sender-attribution
Open

feat(gateway): attribute every inbound sender with stable identity#76516
NicholaiVogel wants to merge 1 commit into
NousResearch:mainfrom
NicholaiVogel:fix/gateway-dm-sender-attribution

Conversation

@NicholaiVogel

@NicholaiVogel NicholaiVogel commented Aug 2, 2026

Copy link
Copy Markdown

What does this PR do?

Adds durable, per-message sender attribution to gateway turns so the agent and persisted transcripts can identify the speaker in every chat context, including DMs. Previously, the per-turn prefix was limited to shared sessions, which left direct messages without durable sender identity.

The implementation uses an adapter-authenticated envelope with a display name and stable platform identity. It preserves Slack's native <@USER_ID> mention target, respects privacy.redact_pii for phone-based platforms, and provides attribute_sender: false for the legacy behavior.

Related Issue

Fixes #35147

Supersedes #13939.

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: prefixes attributed inbound turns with a spoof-resistant sender envelope and preserves the legacy behavior when disabled.
  • gateway/config.py, hermes_cli/config.py: adds default-on attribute_sender, supporting both attribute_sender and gateway.attribute_sender through runtime loading and hermes config set validation.
  • gateway/session.py: shares the platform privacy classification used by sender attribution and session context.
  • tests/gateway/test_shared_group_sender_prefix.py, tests/hermes_cli/test_set_config_value.py: cover DM attribution, canonical IDs, redaction, spoof handling, legacy opt-out, and both config forms.
  • cli-config.yaml.example and website/docs/user-guide/configuration.md: document the setting and privacy behavior using synthetic examples only.

How to Test

  1. Send a Discord DM and confirm the agent receives a sender envelope with the display name and stable user ID before the message text.
  2. Set attribute_sender: false or gateway.attribute_sender: false; confirm direct messages retain legacy unprefixed behavior.
  3. Enable privacy.redact_pii: true for WhatsApp or Signal; confirm sender IDs are stable hashes rather than raw phone-like identifiers.
  4. Run:
    python3 -m pytest -q --basetemp /tmp/hermes-attribution-tests \
      tests/hermes_cli/test_set_config_value.py \
      tests/hermes_cli/test_config_validation.py \
      tests/gateway/test_config.py \
      tests/gateway/test_shared_group_sender_prefix.py \
      tests/gateway/test_session.py \
      tests/gateway/test_pii_redaction.py \
      tests/gateway/test_image_input_routing_runtime.py \
      tests/gateway/test_telegram_noise_filter.py

Validation: 368 focused tests passed locally. scripts/run_tests.sh could not run because this managed checkout's venv lacks pytest; the system Python test runner was used instead.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass (focused suite above: 368 passed)
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Arch Linux

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A (not applicable)
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A (not applicable)

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/gateway Gateway runner, session dispatch, delivery area/config Config system, migrations, profiles needs-decision Awaiting maintainer decision before any implementation 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 sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 2, 2026
@teknium1

teknium1 commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Thanks for carrying the sender identity onto the current inbound turn; current main still limits the prefix to shared sessions (gateway/run.py:15071-15095), while DMs are explicitly excluded from shared-session handling (gateway/session.py:1002-1006). The placement is therefore a useful, cache-safe fix direction.

Problems

  • The documented top-level attribute_sender key is not registered in the CLI config schema. hermes_cli/config.py:1846-1873 enumerates comparable gateway-bridged root keys but omits it; _validate_config_key() reports omitted roots as unknown at hermes_cli/config.py:4739-4746. The PR changes gateway loading, docs, and the example, but not that schema.

Suggested changes

  • Add attribute_sender to the supported config-key schema for the documented top-level and nested forms, then add coverage that hermes config set attribute_sender false and hermes config set gateway.attribute_sender false do not emit an unknown-key warning and load into GatewayConfig.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Aug 2, 2026
@NicholaiVogel

Copy link
Copy Markdown
Author

Thanks @teknium1, I'm addressing your feedback and making a few more adjustments.

@NicholaiVogel
NicholaiVogel force-pushed the fix/gateway-dm-sender-attribution branch from 811aea1 to d823053 Compare August 2, 2026 02:45
@NicholaiVogel

Copy link
Copy Markdown
Author

Addressed in d823053.

  • Registered attribute_sender in the CLI config-key schema, including the documented gateway.attribute_sender form.
  • Added regression coverage that hermes config set accepts both forms without an unknown-key warning and that gateway loading observes the setting.
  • Kept the nested-key allowlist limited to settings the gateway actually reads, so invalid nested paths still warn.
  • Rebased onto current main; focused validation: 368 passed.

@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

One PR addresses #35147. #76516 expands the requested session-context change into per-message attribution, attaching a neutralized display name and adapter-authenticated stable sender ID to attributable inbound turns, including DMs, while preserving platform-specific privacy handling and a legacy opt-out.

Related pull requests

  • feat(gateway): attribute every inbound sender with stable identity #76516 best fix — (+552/-38) — keep open with a salvage path: the diff makes stable sender identity available on every attributable inbound turn through a spoof-resistant envelope, covering the reported DM identity gap more broadly than changing the session-context elif alone. Consistent with the automated keep_open review on feat(gateway): attribute every inbound sender with stable identity #76516, retain the DM/shared-session attribution, forged-envelope stripping, privacy handling, configuration support, and focused tests; the current diff also registers both documented attribute_sender config forms and tests them, directly addressing that review's schema objection.

Suggested consolidation

Keep #76516 open with a salvage path focused on its stable-ID sender envelope, spoofing defenses, privacy behavior, configuration support, and regression coverage. It is the only PR in this complex, so there are no competing PRs to close as duplicates.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I35147(["issue #35147 (open)"])
    P76516["PR #76516 (open)"]
    P76516 -->|best fix| I35147
    class I35147 open
    class P76516 open
    class P76516 best
    class P76516 target
    click I35147 "https://github.com/NousResearch/hermes-agent/issues/35147"
    click P76516 "https://github.com/NousResearch/hermes-agent/pull/76516"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 1 pull request and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 31 kB of PR diffs, 8 kB of issue/PR text, <1 kB of discussion (2 comments), 2 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

@alt-glitch alt-glitch added comp/cli CLI entry point, hermes_cli/, setup wizard and removed sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Aug 4, 2026
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 comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit 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 sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Expose user_id alongside user_name in single-user session context

4 participants