Skip to content

fix(gateway): complete agent:start/end hook symmetry on interrupt/drain turns - #39126

Open
Couiz wants to merge 3 commits into
NousResearch:mainfrom
Couiz:pr/hook-symmetry
Open

fix(gateway): complete agent:start/end hook symmetry on interrupt/drain turns#39126
Couiz wants to merge 3 commits into
NousResearch:mainfrom
Couiz:pr/hook-symmetry

Conversation

@Couiz

@Couiz Couiz commented Jun 4, 2026

Copy link
Copy Markdown

Why this matters

The gateway emits agent:start / agent:end hooks so integrations can observe the lifecycle of every turn — activity loggers, session trackers, turn visualizers, "agent is thinking" indicators, and anything that pairs a start with its end. These hooks are only useful if they fire reliably and in balanced pairs. Today they don't, on the one path that matters most for responsiveness: the interrupt/drain follow-up.

When a user sends a second message while the agent is still working, the gateway interrupts the current turn and drains the queued message into a recursive follow-up turn. That follow-up is exactly when a hook consumer most wants to know "a new turn started / ended" — but the follow-up re-enters _run_agent recursively instead of the outer _handle_message_with_agent, so its lifecycle events were getting lost.

Concrete example (our use case): we run a agent:start hook that captures and re-emits the transcribed text of a voice message so downstream tooling sees what the user actually said — and it must fire even when the message arrives as an interrupt mid-turn. Without these fixes, an interrupting voice message produced a turn with no observable start (so the transcription was never surfaced) and an unbalanced event stream. Any hook that does work on turn boundaries — not just ours — hits the same gap.

What this PR does (three coherent changes, one concern)

This is a single, self-contained piece of work: make agent:start/agent:end symmetric and well-tagged on interrupt/drain turns. It's presented as three reviewable commits:

  1. fix: emit agent:start on interrupt/drain follow-up turns — the drained follow-up turn now fires agent:start (it previously fired none), so hooks observing turn starts no longer silently miss follow-ups. Includes the voice-follow-up case: the start carries the transcribed text, not raw audio.
  2. feat: tag agent:start payload with trigger + interrupt_depth — every agent:start now carries trigger ("message" for a normal turn, "interrupt" for a drained follow-up) and interrupt_depth, so consumers can distinguish and attribute turns. trigger is a string (not a bool) so future turn kinds extend the contract without breaking existing consumers, which read ctx.get("trigger", "message").
  3. fix: emit agent:end on interrupt/drain follow-up turns — completes the symmetry: the drained follow-up now fires a matching agent:end (mirroring the main-path end payload), so start/end-pairing hooks stay balanced on every interrupt. The end emit sits below the discard and _MAX_INTERRUPT_DEPTH early-returns, so paths that emit no start also emit no end — symmetry holds on every branch.

Correctness

  • Empirically verified, not assumed. The asymmetry (one extra agent:start per interrupt, no matching agent:end) was confirmed by instrumenting the real _run_agent drain path before any production change.
  • RED→GREEN. New regression tests fail against base for the right reason (end count < start count; no start on the drain path) and pass after the fix.
  • Tests (4 files, +677, no production-line churn beyond the emits):
    • test_drain_emits_agent_start.py — start fires on text + voice follow-ups; voice emits transcribed text not audio; 500-char truncation; trigger/depth tagging; nested-interrupt depth increments.
    • test_agent_start_trigger.py — main dispatch is trigger="message", depth=0.
    • test_drain_agent_end_symmetry.py — start/end balanced on a single interrupt; end payload mirrors start; nested-depth end increments; no end at _MAX_INTERRUPT_DEPTH and neither emitted on the two discard paths (proves symmetry on early-return branches).
  • No contract breakage: the agent:start payload gains keys; existing consumers read by .get() and reference the event by name. Main-path start/end, _MAX_INTERRUPT_DEPTH, and history-offset preservation are untouched.
  • Full tests/gateway/ suite shows zero new failures attributable to this change.

Notes for the reviewer

Couiz and others added 3 commits June 4, 2026 14:32
The main message dispatch emits the `agent:start` hook before running the
agent, but the interrupt/drain follow-up path in `_run_agent` promoted a
queued message straight into a recursive `_run_agent` call without emitting
`agent:start`. Every hook listening on `agent:start` (SessionStart-style
integrations, activity loggers, visualizers) silently missed interrupt/queue
follow-up turns — an event-emission gap, not a hook bug.

Emit `agent:start` on the drain path right before the recursive `_run_agent`,
mirroring the main-dispatch payload (platform, user_id, chat_id, session_id,
message[:500]) but built from the follow-up turn's source (`next_source`) and
the final, already-transcribed text (`next_message`) — so voice follow-ups
carry the transcript, not the raw audio placeholder. The emit sits after every
discard guard (draining, interrupt depth-cap, stale /goal continuation,
transcription→None) so it fires exactly once per turn, only when the follow-up
actually proceeds to the agent.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
(cherry picked from commit 34bb1cc)
(cherry picked from commit f88f2dd104e705a1ffaf0b825f2af10ad27c2cc6)
…depth

Both agent:start emit sites — the main inbound dispatch and the
interrupt/drain follow-up path in _run_agent — previously emitted an
identical payload shape, so hooks (voice-echo, activity loggers,
visualizers) could not tell a fresh user turn from an interrupt-driven
follow-up turn.

Add two discriminator fields to the agent:start payload at both sites:

  * trigger — a string, not a bool, so future turn kinds like "goal" or
    "schedule" can be added without breaking the contract: "message" on the
    main dispatch, "interrupt" on the drain follow-up. Hooks read it
    backward-compatibly as context.get("trigger", "message").
  * interrupt_depth — an int: 0 on the main dispatch (a fresh turn is never
    an interrupt); _interrupt_depth + 1 on the drain path, matching the
    depth handed to the recursive _run_agent call (first interrupt -> 1,
    interrupt-of-an-interrupt -> 2, ...).

Both payloads stay shape-consistent (7 keys); no existing key changes. The
drain emit still sits after every discard guard, so it fires once per turn
only when the follow-up actually proceeds to the agent.

Depends on NousResearch#37269.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
(cherry picked from commit b2600ec)
(cherry picked from commit 451386527c8badc9e7e3fa9c3fc8c17ffddc59a0)

@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 isolating a real interrupt/drain lifecycle gap. Current main still emits hooks only around the outer dispatch (gateway/run.py:11620-11630, :11841-11845) while the drained follow-up recurses directly at gateway/run.py:19953-19964.

Problems

  • The new followup_hook_ctx omits thread_id and chat_type. Those are part of the current documented agent:start/agent:end context (gateway/hooks.py:21-32) and the outer dispatch supplies them (gateway/run.py:11621-11629). A drained forum/topic turn would lose that metadata.
  • The drain path has moved substantially since this PR's base: current main resolves next_session_key, runs profile-scoped preparation, and refreshes the cache snapshot before recursion (gateway/run.py:19886-19964). The salvage should place the emits around that current recursion and test that path.

Suggested changes

  • Preserve the full current hook context on drained start/end events, including thread_id and chat_type, and cover a topic follow-up.
  • Document the additive trigger and interrupt_depth fields in gateway/hooks.py.

Automated hermes-sweeper review.

Comment thread gateway/run.py
# once and reuse it for the paired agent:end below — exactly as
# the main path reuses `hook_ctx` for both start and end.
followup_hook_ctx = {
"platform": next_source.platform.value if next_source.platform else "",

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 include thread_id and chat_type here as well. Current agent:start/agent:end consumers are documented to receive both (gateway/hooks.py:21-32), and the outer dispatch already provides them; otherwise drained forum/topic turns lose routing metadata.

@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 labels Jul 14, 2026

@GottZ GottZ left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This was generated by AI during triage.

Summary

Two PRs address the same interrupt/drain lifecycle gap with effectively identical diffs: both add balanced agent:start/agent:end emissions around the recursive follow-up and add trigger/interrupt_depth discrimination, but both are based on an older drain path and omit current hook-context fields.

Related pull requests

  • #38915 [closed] duplicate — (+677/-0) — superseded duplicate: Adds drained follow-up start/end symmetry, response propagation, and interrupt-depth tagging with regression coverage; it remains relevant as the closed predecessor that was explicitly consolidated into #39126, and its diff is effectively identical to #39126.
  • #39126 related — (+677/-0) — keep open for salvage, not merge-ready: The diff directly fixes the missing lifecycle hooks around recursive drain turns, but, as the contributor keep_open review documents, it omits thread_id and chat_type, targets a drain path that has since moved, and lacks topic-follow-up coverage and hook-field documentation.

Duplicates

#38915 and #39126 contain effectively the same production changes and test additions; #38915 was closed as superseded by #39126.

Suggested consolidation

Merge #39126 only after rebasing the hook emissions around the current recursive drain path, preserving the full documented context including thread_id and chat_type, adding topic-follow-up coverage, and documenting the additive discriminator fields. This follows the contributor keep_open review rather than overriding it; #38915 can remain closed as the duplicate predecessor superseded by #39126.

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 Dup38915 ["PRs duplicating each other"]
        P38915["PR #38915 (closed)"]
        P39126["PR #39126 (open)"]
    end
    class P38915 closed
    class P39126 open
    class P39126 target
    click P38915 "https://github.com/NousResearch/hermes-agent/pull/38915"
    click P39126 "https://github.com/NousResearch/hermes-agent/pull/39126"
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 or no verify verdict yet (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: 53 kB of PR diffs, 7 kB of issue/PR text, 2 kB of discussion (2 comments), 1 verify verdict. 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

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.

4 participants