Skip to content

feat(webhook): per-route session mode — allow shared persistent sessions - #71570

Open
jzOcb wants to merge 1 commit into
NousResearch:mainfrom
jzOcb:feat/webhook-shared-session
Open

feat(webhook): per-route session mode — allow shared persistent sessions#71570
jzOcb wants to merge 1 commit into
NousResearch:mainfrom
jzOcb:feat/webhook-shared-session

Conversation

@jzOcb

@jzOcb jzOcb commented Jul 25, 2026

Copy link
Copy Markdown

Problem

Webhook routes derive the agent session key from the delivery id (gateway/platforms/webhook.py):

session_chat_id = f"webhook:{route_name}:{delivery_id}"

so every delivery spawns a brand-new, zero-context session. That is the right default for independent event streams (CI pings, monitoring alerts), but the wrong isolation granularity for conversational/workflow webhook consumers — e.g. an agent-to-agent relay where each message continues one ongoing collaboration.

Observed in production use (multi-agent setup, one route, serial messages):

  • every delivery cold-starts: the session re-reads project state and searches past sessions to rebuild context (slow, token-expensive, and the reconstruction is fallibly lossy);
  • decisions/approvals given to the agent on a chat platform are invisible to webhook-spawned sessions, which then re-request them from the operator;
  • behavioural consistency (rules adopted mid-conversation) resets per delivery.

Change

Opt-in, backward-compatible per-route setting in the webhook subscription:

"session": "shared"

Default "per-delivery" preserves current behaviour exactly. With "shared", the session key omits the delivery id (webhook:{route_name}), so all deliveries on that route continue one persistent session. Deliveries on a shared route serialize into that session's queue rather than running concurrently — which is the desired semantics for workflow routes.

Delivery-id dedup, HMAC verification, and delivery-info bookkeeping are unchanged (delivery info is keyed by the session chat id; on a shared route later deliveries refresh the same entry — same route, same target, no behavioural change to send()).

Related

Filing a separate issue for an adjacent defect this surfaced: /approve is session-scoped while approval requests are delivered cross-platform, making dangerous-command gates in webhook sessions unapprovable. This PR reduces its blast radius (a shared session keeps pending state reachable across deliveries) but does not fix the routing itself.

🤖 Generated with Claude Code

Webhook routes derive the agent session key from the delivery id, so every
delivery spawns a fresh zero-context session. Right default for independent
event streams; wrong isolation granularity for conversational/workflow
consumers (e.g. agent-to-agent relays), which cold-start on every message,
lose in-conversation decisions, and re-request approvals.

Adds an opt-in, backward-compatible per-route setting:

    "session": "shared"    (default: "per-delivery", current behavior)

With "shared" the session key omits the delivery id, so all deliveries on
the route continue one persistent session; deliveries serialize into its
queue rather than running concurrently — the desired semantics for workflow
routes. Delivery-id dedup, HMAC verification, and delivery-info bookkeeping
are unchanged.
@jzOcb

jzOcb commented Jul 25, 2026

Copy link
Copy Markdown
Author

The adjacent /approve session-scope defect mentioned in the description is now filed as #71571.

@alt-glitch alt-glitch added type/feature New feature or request comp/gateway Gateway runner, session dispatch, delivery platform/webhook Webhook / API server P3 Low — cosmetic, nice to have sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 25, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #57972. Both make webhook sessions persistent, but this PR selects one shared session per route while #57972 uses a rendered per-payload session_key and includes lifecycle/fallback handling; maintainer policy choice is needed.

@jzOcb jzOcb mentioned this pull request Jul 25, 2026
16 tasks

@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 opt-in design. The per-delivery premise is present on current main at gateway/platforms/webhook.py:872-874, but this patch needs lifecycle and delivery-routing work before it can provide a persistent shared session.

Problems

  • gateway/platforms/webhook.py:936-958 still closes every completed webhook session. On the next delivery, gateway/session.py:2317-2388 detects the webhook_complete row as stale and recreates it, so the new key alone does not preserve history.
  • The route-scoped key also aliases _delivery_info: writes occur at gateway/platforms/webhook.py:885-887, while send() resolves the destination from that key at gateway/platforms/webhook.py:375-376. A later delivery can therefore replace the earlier response's rendered delivery target.

Suggested changes

  • Separate durable conversation identity from per-delivery response-routing state, and exempt only shared routes from the per-delivery close path while retaining bounded lifecycle handling.
  • Add real dispatch-pipeline tests for serial shared deliveries and overlapping deliveries with different rendered deliver_extra values; document the new route setting in website/docs/user-guide/messaging/webhooks.md:79-90.

Automated hermes-sweeper review.

# Right for workflow/relay routes where each event continues
# the same collaboration.
session_scope = str(route_config.get("session") or "per-delivery").strip().lower()
if session_scope == "shared":

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 changes only the routing key, but on_processing_complete() still unconditionally ends every webhook session at current gateway/platforms/webhook.py:936-958. The next delivery sees that webhook_complete row as stale and receives a fresh session through SessionStore (gateway/session.py:2317-2388), so shared routes will not retain transcript history without a corresponding lifecycle change.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages 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 30, 2026
@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

Two open PRs address persistent webhook conversations: #57972 adds rendered per-payload session identities, fallback behavior, lifecycle handling, and documentation, while #71570 adds one shared session per route. Neither diff safely resolves the cause because conversation identity remains coupled to mutable response-routing state; #57972 also mishandles fallback lifecycle, and #71570 leaves completion-time auto-close unchanged.

Related pull requests

Duplicates

#57972 and #71570 substantially duplicate the opt-in persistent-webhook-session capability; #71570 is the constant-per-route variant, while #57972 additionally supports rendered per-payload conversation identities.

Suggested consolidation

Keep #57972 open with a salvage path: separate stable conversation identity from per-delivery response routing and skip auto-close only when that event successfully resolves a persistent key. Keep #71570 open while that correction is pending; once #57972 demonstrably covers the constant-per-route case and both contributor review blockers, #71570 can be closed as its narrower duplicate.

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
    subgraph Dup57972 ["PRs duplicating each other"]
        P57972["PR #57972 (open)"]
        P71570["PR #71570 (open)"]
    end
    class P57972 open
    class P71570 open
    class P71570 target
    click P57972 "https://github.com/NousResearch/hermes-agent/pull/57972"
    click P71570 "https://github.com/NousResearch/hermes-agent/pull/71570"
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 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 7 kB of PR diffs, 11 kB of issue/PR text, 4 kB of discussion (6 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

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 platform/webhook Webhook / API server 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-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.

4 participants