Skip to content

feat(sessions): expose session_key in GET /api/sessions - #46165

Open
cgart wants to merge 2 commits into
NousResearch:mainfrom
cgart:feature/sessions-api-session-key
Open

cgart wants to merge 2 commits into
NousResearch:mainfrom
cgart:feature/sessions-api-session-key

Conversation

@cgart

@cgart cgart commented Jun 14, 2026

Copy link
Copy Markdown

What does this PR do?

Exposes each session's gateway binding through GET /api/sessions: session_key
(the stable routing key, e.g. agent:main:telegram:group:<chat_id>:<thread_id>)
plus the structured chat_id, chat_type, and thread_id fields.

Since #54442 these fields are already persisted in the sessions table and
returned by list_sessions_rich — but _session_response strips them, so API
consumers still can't see them. This PR adds them to the client-safe allowlist
(one line); system_prompt/model_config stay redacted to existence flags.
All four fields are null for sessions without a gateway origin.

Why / what this is for

I'm building an external client that drives Hermes over its HTTP API. It needs
session persistency across restarts and must inject its turns into the right
live Telegram session — the one bound to a specific chat, group, or topic
thread. user_id alone can't express that binding: it's the sender's personal
ID and is identical across DMs and group messages. With the gateway binding
exposed, the client can enumerate sessions over REST and deterministically
correlate each one to its channel instead of guessing.

Type of Change

  • ✨ New feature (non-breaking change that adds functionality)

Changes Made

  • gateway/platforms/api_server.py — add session_key, chat_id, chat_type,
    thread_id to the _session_response safe-key allowlist
  • tests/ — allowlist unit tests + end-to-end GET /api/sessions test
    (including null passthrough for rows without a gateway origin)
  • website/docs/user-guide/features/api-server.md — document the fields
  • scripts/release.py — contributor email mapping for the attribution check

How to Test

  1. scripts/run_tests.sh tests/gateway/test_api_server_normalize.py tests/hermes_cli/test_web_server.py
  2. Create a session with a gateway origin, then GET /api/sessions — the
    binding fields are present; CLI sessions return null.

Checklist

  • Conventional Commits, focused diff, tests added and passing, ruff clean
  • Docs updated
  • No cross-platform impact — N/A (pure dict allowlist)

@cgart
cgart force-pushed the feature/sessions-api-session-key branch 2 times, most recently from ffacde9 to 7b96650 Compare June 14, 2026 14:26
@alt-glitch alt-glitch added type/feature New feature or request comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have labels Jun 14, 2026
@cgart
cgart force-pushed the feature/sessions-api-session-key branch from 7b96650 to 64ac295 Compare June 14, 2026 15:20
@cgart
cgart marked this pull request as draft June 14, 2026 15:28
@cgart
cgart marked this pull request as ready for review June 14, 2026 15:32
@cgart
cgart force-pushed the feature/sessions-api-session-key branch 5 times, most recently from 7e07f8b to ce4e7cd Compare June 16, 2026 06:01
@keslerm

keslerm commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Ran into this exact issue today, would love to get this merged for sure

cgart pushed a commit to cgart/hermes-agent that referenced this pull request Jun 25, 2026
…a session (NousResearch#28746)

The _session_expiry_watcher sets expiry_finalized=True and evicts the cached
agent when an idle session exceeds its reset policy, but it never called
db.end_session() — so ended_at stayed NULL in SQLite indefinitely.

GET /api/sessions filters on ended_at IS NULL to find live sessions, meaning
external API clients kept treating a finalized session as live and injecting
turns into a stale conversation that the gateway had already discarded
internally. This surfaced when building on top of the session API extended
in NousResearch#46165: polling /api/sessions by session_key suffix correctly resolves the
most-recently-active session, but when the gateway has silently expired it via
the watcher, the stale session still appears live — ended_at: null — causing
clients to keep addressing it instead of letting Hermes create a fresh one on
the next turn.

Fix: after setting expiry_finalized=True, call session_store._db.end_session()
with reason 'idle'. This matches the reason used by get_or_create_session on
the next-message path so the two paths are consistent. The call is guarded by
getattr so a missing _db (fallback JSONL mode) is a no-op.

Add a regression test to test_session_boundary_hooks.py alongside the
existing NousResearch#14981 test to verify the DB write happens on idle expiry.
@cgart
cgart force-pushed the feature/sessions-api-session-key branch from 44423c4 to 8e907b0 Compare June 28, 2026 09:20
@cgart

cgart commented Jun 28, 2026

Copy link
Copy Markdown
Author

Rebased on latest main — the branch is now conflict-free (single clean commit on top of current HEAD).

@teknium1 would appreciate a review when you have a moment. There's also at least one other user (@keslerm) who ran into the same gap, so the use case seems broader than just my setup.

@cgart
cgart force-pushed the feature/sessions-api-session-key branch from 8e907b0 to 694529b Compare July 7, 2026 15:08

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the focused API-surface fix. The current main implementation still strips these persisted routing fields in gateway/platforms/api_server.py:1681-1694, so the core allowlist change is needed.

Problems

  • website/docs/user-guide/features/api-server.md:339 shows a sessions response member, while the API-server endpoint actually emits its rows under data in gateway/platforms/api_server.py:1754-1760. Please update the example envelope.

Suggested changes

  • The new integration test in tests/hermes_cli/test_web_server.py:670 covers the dashboard's separate /api/sessions route, not the changed adapter serializer. Add an aiohttp route-level assertion in tests/gateway/test_api_server.py; its _create_app() helper is at :618-634 and can register _handle_list_sessions.

Automated hermes-sweeper review.

# list sessions, including each session's gateway binding
curl "http://localhost:8642/api/sessions?limit=20" \
-H "Authorization: Bearer $API_SERVER_KEY"
# → {"sessions": [

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The API-server handler returns list rows under data, not sessions (gateway/platforms/api_server.py:1754-1760). Please make this example match the actual response envelope.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Adopted

Comment thread tests/hermes_cli/test_web_server.py Outdated
finally:
db.close()

rows = self.client.get("/api/sessions?limit=20&offset=0").json()["sessions"]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This exercises the dashboard FastAPI endpoint, which returns raw SessionDB rows and never invokes the changed APIServerAdapter._session_response. Please add the route-level API-server assertion in tests/gateway/test_api_server.py as well.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done, hopefully the right way. Thanks for pointing out

@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 labels Jul 14, 2026
@cgart
cgart force-pushed the feature/sessions-api-session-key branch 2 times, most recently from a6e4b5a to 2fd7d4a Compare July 14, 2026 19:05
@cgart

cgart commented Jul 14, 2026

Copy link
Copy Markdown
Author

Rebased on latest main state as well

@teknium1 teknium1 added the area/sessions Session lifecycle, resume, persistence, history label Jul 19, 2026
cgart added 2 commits August 29, 2026 05:26
Add session_key, chat_id, chat_type, and thread_id to the client-safe
field allowlist in _session_response, so external API consumers can map
a live session to the specific chat/group/thread it originated from
without inferring it from user_id alone (for Telegram, user_id is the
sender's personal ID and is identical across DMs and group messages).

The fields themselves are already persisted since the session-restart
work in NousResearch#54442 and returned by list_sessions_rich — the API layer just
stripped them. Sensitive snapshots (system_prompt, model_config) remain
redacted to existence flags.

Tests cover the allowlist at the _session_response unit level and
through the aiohttp GET /api/sessions route (envelope + binding fields
+ NULL passthrough for rows without a gateway origin).
Describe the session_key / chat_id / chat_type / thread_id fields now
returned per session by GET /api/sessions, with a response example
matching the actual list envelope ({"object": "list", "data": [...]}),
and cross-link the inbound X-Hermes-Session-Key header that carries the
same routing key.
@cgart
cgart force-pushed the feature/sessions-api-session-key branch from 2fd7d4a to 98b416b Compare August 29, 2026 05:26
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 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/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants