Skip to content

fix(gateway): add chat_id to session:start, session:end, and session:reset hook contexts - #24982

Open
AhmetArif0 wants to merge 1 commit into
NousResearch:mainfrom
AhmetArif0:fix/gateway-session-hooks-add-chat-id
Open

fix(gateway): add chat_id to session:start, session:end, and session:reset hook contexts#24982
AhmetArif0 wants to merge 1 commit into
NousResearch:mainfrom
AhmetArif0:fix/gateway-session-hooks-add-chat-id

Conversation

@AhmetArif0

Copy link
Copy Markdown
Contributor

What does this PR do?

Root cause: _handle_message_with_agent() emits session:start and _handle_reset_command() emits session:end and session:reset, but none of them included chat_id in the hook context. Hook authors who subscribe to session lifecycle events cannot identify which conversation the session belongs to, making routing (e.g. sending a welcome message on session:start or cleanup on session:end) impossible without it.

Fix: mirror the "chat_id": source.chat_id or "" field added to agent:start by #24710 into the three sibling session hooks. source is in scope at all three emit sites and chat_id is a required str field on SessionSource, so no guard is needed beyond the existing or "" fallback.

Type of Change

  • 🐛 Bug fix

Changes Made

  • gateway/run.py: add "chat_id" to session:start (+1 line), session:end (+1 line), session:reset (+1 line) hook contexts
  • tests/gateway/test_session_boundary_hooks.py: two new tests asserting chat_id is present in session:end and session:reset contexts

How to Test

pytest tests/gateway/test_session_boundary_hooks.py -v --override-ini="addopts="

Checklist

  • Contributing Guide read | Conventional Commits | No duplicate PR
  • Single logical change | Tests added | Platform: macOS
  • Docs — N/A | Cross-platform — N/A

…reset hook contexts

_handle_message_with_agent() emits session:start and _handle_reset_command()
emits session:end and session:reset, but none of them included chat_id in the
hook context. Hook authors who subscribe to session lifecycle events cannot
identify which conversation the session belongs to, making routing (e.g.
sending a welcome message on session:start or cleanup on session:end) impossible.

Fix: mirror the "chat_id": source.chat_id or "" field added to agent:start
by NousResearch#24710 into the three sibling session hooks. source is in scope at all
three emit sites and chat_id is a required str field on SessionSource.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery labels May 13, 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 the focused lifecycle-hook fix. The omission is still present on current main: session:start lacks chat_id at gateway/run.py:10842-10847, and session:end / session:reset lack it at gateway/slash_commands.py:238-250.

Problems

  • The PR's two reset edits target the pre-refactor location in gateway/run.py; _handle_reset_command moved to gateway/slash_commands.py in 619bd7827, so those hunks need to be applied there.
  • tests/gateway/test_session_boundary_hooks.py adds coverage for session:end and session:reset, but not the new session:start field.
  • The public event-context table at website/docs/user-guide/features/hooks.md:78-80 does not yet describe chat_id for these lifecycle events.

Suggested changes

  • Move the reset payload additions to gateway/slash_commands.py:239-250 and add a session:start context assertion.
  • Document the additive chat_id key for all three lifecycle events.

Automated hermes-sweeper review.

Comment thread gateway/run.py
await self.hooks.emit("session:start", {
"platform": source.platform.value if source.platform else "",
"user_id": source.user_id,
"chat_id": source.chat_id or "",

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.

Please add a companion regression test for this session:start payload. The new tests cover session:end and session:reset, but not this third added field.

@teknium1 teknium1 added 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 area/sessions Session lifecycle, resume, persistence, history labels Jul 13, 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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants