Skip to content

Label unnamed shared-session participants - #44705

Open
OmarB97 wants to merge 3 commits into
NousResearch:mainfrom
OmarB97:codex/channel-participant-fanout-harness-20260612
Open

Label unnamed shared-session participants#44705
OmarB97 wants to merge 3 commits into
NousResearch:mainfrom
OmarB97:codex/channel-participant-fanout-harness-20260612

Conversation

@OmarB97

@OmarB97 OmarB97 commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add a shared-session participant label helper that preserves display names when present and falls back to a stable redacted sender label when no display name is available
  • sanitize user-controlled display labels before they are placed in [label] prefixes so aliases stay single-line and bracket-safe
  • prefix inbound shared multi-user session messages with that sender label so multiple unnamed humans/devices remain distinguishable without leaking raw opaque IDs
  • update the shared-session context prompt from [sender name] to [sender label]
  • add a session event fanout harness proving a primary transport and sidecar listener receive the same JSON-RPC session event frame
  • map omar@kostudios.io in scripts/release.py so contributor attribution CI can resolve the PR author

Verification

  • python3 -m pytest tests/gateway/test_shared_group_sender_prefix.py tests/gateway/test_session.py tests/tui_gateway/test_protocol.py -q (144 passed)
  • python3 -m compileall -q scripts/release.py gateway/session.py gateway/run.py tests/gateway/test_shared_group_sender_prefix.py tests/tui_gateway/test_protocol.py
  • uv tool run ruff check scripts/release.py gateway/session.py gateway/run.py tests/gateway/test_shared_group_sender_prefix.py tests/gateway/test_session.py tests/tui_gateway/test_protocol.py
  • git diff --check
  • git diff upstream/main -- gateway/session.py gateway/run.py tests/gateway/test_shared_group_sender_prefix.py tests/gateway/test_session.py tests/tui_gateway/test_protocol.py scripts/release.py | rg -n "discord|Discord|telegram|Telegram" (no matches)
  • portable local attribution-map emulation for PR author emails (passed)

Risk

The behavior change is limited to shared multi-user sessions. Per-user isolated sessions keep their existing plain inbound message behavior, and raw sender IDs are not exposed in the fallback label. Display aliases are preserved after whitespace/bracket normalization.

@alt-glitch alt-glitch added type/feature New feature or request P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery labels Jun 12, 2026
@liuhao1024

Copy link
Copy Markdown
Contributor

Verified clean — shared session participant labeling

Reviewed the diff across gateway/session.py, gateway/run.py, and tests:

  1. Bracket injection prevention: _sanitize_participant_label replaces [/] with (/) and truncates to 64 chars. This prevents user-controlled display names from breaking the [label] message prefix format.

  2. Stable fallback: When no display name exists, shared_participant_label falls back to _hash_sender_id(raw) which produces a deterministic user_XXXXXXXX label. Different user_id values produce different labels (verified by test_shared_participant_labels_distinguish_multiple_unnamed_senders).

  3. Priority chain: user_nameuser_id_altuser_idNone. The user_id_alt check before user_id handles platforms that provide an opaque internal ID alongside a human-readable one.

  4. No behavioral regression: The prefix format change from [sender name] to [sender label] is cosmetic in the session context prompt — tests updated to match.

No findings.

@OmarB97

OmarB97 commented Jul 11, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough verification @liuhao1024 — that write-up matches the implementation exactly, and I appreciate the injection-prevention check in particular.

Re-confirmed today (2026-07-11) at the current head (c55ce3d4): _sanitize_participant_label still collapses whitespace, swaps brackets for parens, and caps at 64 chars (gateway/session.py:57); the fallback chain in shared_participant_label is display name → user_id_altuser_id → None with the deterministic user_<hash> label (gateway/session.py:165). I also checked that current upstream main still has none of this labeling, so the PR is not superseded — it remains mergeable as-is against today's main.

Ready for a maintainer merge whenever it fits.

@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 addressing the real unnamed-participant gap: current main only prefixes a shared turn when source.user_name is present (gateway/run.py:10388-10389).

Problems

  • The new fallback at gateway/run.py:7627 also applies to Yuanbao group turns. Yuanbao builds a source with user_id and thread_id="main" (gateway/platforms/yuanbao.py:2010-2016), then deliberately preformats the message as [nickname|user_id] and clears only user_name to suppress the runner prefix (gateway/platforms/yuanbao.py:2160-2164). The fallback will therefore add [user_<hash>] before that existing label.

Suggested changes

  • Preserve an explicit no-second-prefix signal for already-attributed adapter messages while retaining the opaque-ID fallback for ordinary shared turns.
  • Add a Yuanbao group/thread regression proving the message remains singly attributed.

Automated hermes-sweeper review.

Comment thread gateway/run.py Outdated
if _is_shared_multi_user and source.user_name:
message_text = f"[{source.user_name}] {message_text}"
if _is_shared_multi_user:
sender_label = shared_participant_label(source)

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.

This fallback also defeats Yuanbao's intentional suppression: its group middleware preformats [nickname|user_id] and clears only source.user_name to prevent the runner prefix (gateway/platforms/yuanbao.py:2160-2164), while retaining user_id. This will now produce [user_<hash>] [nickname|user_id] ...; preserve an explicit already-attributed/no-prefix path and add a regression.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
@teknium1 teknium1 added the area/sessions Session lifecycle, resume, persistence, history label Jul 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists 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-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.

4 participants