Skip to content

feat(webhook): add opt-in persistent session keys - #90589

Open
jzOcb wants to merge 8 commits into
NousResearch:mainfrom
jzOcb:feat/webhook-persistent-session-key-v2
Open

feat(webhook): add opt-in persistent session keys#90589
jzOcb wants to merge 8 commits into
NousResearch:mainfrom
jzOcb:feat/webhook-persistent-session-key-v2

Conversation

@jzOcb

@jzOcb jzOcb commented Aug 20, 2026

Copy link
Copy Markdown

Summary

Adds an opt-in per-route session_key template so conversational webhook sources can reuse a Hermes session instead of creating a fresh delivery-ID session for every POST. Default behavior remains per-delivery.

This supersedes the stale/conflicting implementation path in #57972 and covers the narrower constant-route proposal in #71570. It intentionally does not address cross-session approval routing (#71571).

Safety and behavior

  • A successfully rendered session_key creates a persistent conversation identity; unresolved templates safely fall back to the existing one-shot delivery session and still auto-close.
  • Persistent (session:<key>) and fallback (delivery:<delivery-id>) identities use disjoint namespaces, so a delivery ID cannot collide with, reuse, or auto-close a persistent conversation.
  • Conversation identity is separate from delivery routing: each delivery retains its own rendered deliver_extra, preventing concurrent turns from redirecting another turn’s response.
  • Persistent deliveries use the gateway FIFO rather than the adapter’s single pending slot, so multiple arrivals keep their own turns, delivery IDs, and response targets without text merging, interrupting, or steering.
  • Multiplexed profiles remain separate even if the route and rendered key match.
  • Documentation covers configuration, fallback, lifecycle, ordering, isolation, and sender-controlled key boundaries.

Validation

python -m pytest -o addopts= -q \
  tests/gateway/test_webhook_integration.py \
  tests/gateway/test_webhook_session_close.py \
  tests/gateway/test_multiplex_busy_input_mode.py \
  tests/gateway/test_webhook*.py
# 85 passed

git diff --check
python -m compileall -q gateway/platforms/webhook.py gateway/run.py

The full tests/gateway suite was also exercised locally. The targeted webhook coverage is green; remaining failures are existing macOS/environmental Discord/AF_UNIX/DNS isolation failures outside this diff.

Credit

This branch preserves the authored commits from Gilles Gameiro and Atroci; the additional commits add profile-isolation, multi-delivery FIFO, and persistent-vs-fallback namespace-collision regression coverage.

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/gateway Gateway runner, session dispatch, delivery platform/webhook Webhook / API server 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 20, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference; please use your judgment.

Excellent feature design: session_key cleanly separates conversation identity from delivery identity, per-delivery deliver_extra storage means an overlapping second POST cannot redirect an in-flight turn's response target (proven by the two-event interleaving test with exact send args), the busy-input path deliberately FIFOs persistent deliveries instead of interrupting/merging, fallback one-shot deliveries keep their own lifecycle including session-close, profile multiplexing namespaces automatically, and the docs' behavior-notes section (idempotency unchanged, sender-controlled keys scoped to route+HMAC, ordering-vs-concurrency guidance) is genuinely thorough.

  1. gateway/platforms/webhook.py (send resolution chain, ~380–386) — for persistent turns the chain is ContextVar delivery-id → reply_tochat_id, but chat_id is now webhook:<route>:session:<key> which is never a key in _delivery_info — why it matters: any send() that runs outside the ContextVar's async context and without reply_to (e.g., an interim status emitted from a different task, or after context teardown) finds nothing and silently downgrades to deliver="log" — suggestion: enumerate the gateway's send paths for webhook sessions and add a test where the final response fires from a fresh task; alternatively fall back to scanning _delivery_info_order for this session's most recent entry.

  2. ~928–931 — unresolved-template detection is "{" in session_key; a successfully rendered key whose value legitimately contains { also degrades to one-shot — safe direction, but worth documenting next to the fallback bullet since it's non-obvious.

  3. Nit (:~1003–1005): _active_delivery_id.set(...) is never reset in on_processing_complete; task contexts are usually discarded, but a pooled/reused task would carry a stale delivery id into an unrelated turn — consider resetting with the ContextVar token in a finally.

— reviewer-a · automated agent review (Hermes week-review)

@jzOcb

jzOcb commented Aug 22, 2026

Copy link
Copy Markdown
Author

Thanks — I checked the send paths and addressed the actionable items in 8da05c6.

  • Added a regression that sends from a child asyncio.create_task() during each active persistent turn. Task context propagation retains the delivery ID, and the test proves both status and final responses reach their own rendered targets under overlapping turns.
  • Restored the delivery ContextVar with its token in on_processing_complete (including the persistent-session early return), with regression coverage for restoring an inherited prior context.
  • Documented the conservative { fallback behavior for rendered keys.

A task created after a run has completed has no turn/delivery identity to route safely; selecting the newest delivery for that persistent conversation would reintroduce the cross-turn misrouting this PR prevents, so that case intentionally remains log-only rather than guessing.

Validation: python -m pytest -o addopts= -q tests/gateway/test_webhook*.py → 70 passed; focused integration/session-close/busy-mode set → 27 passed; compile and git diff --check passed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have platform/webhook Webhook / API server 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.

5 participants