Files
hermes-agent/gateway/run_turn_followup_ack.py
Mira Solari 606ea7f4ff fix(gateway): fire processing hooks for runner-drained queued follow-ups (#72502, salvage #72503)
A message that arrives mid-turn is parked in the adapter's pending slot and drained in-band
by the runner's recursive _run_agent, a path that never touches on_processing_start /
on_processing_complete — so queued, interrupting and steer-demoted messages never got the
read-receipt reaction idle-session messages get, on every adapter implementing the hooks.

Bracket the drain in TurnRunner._run_agent_queued_followup with the hooks, resolving the
adapter from the follow-up's own source (multiplex-safe). Fires only for real inbound
platform events (message_id or raw_message present) and only for adapters that override
on_processing_start, so complete-only adapters (Google Chat, webhook) are not handed an
early completion. Cancels are classified like _process_message_background does.

Re-ported onto current main: the drain moved from gateway/run.py to
gateway/run_turn.py::_run_agent_queued_followup (#102117 decomposition) and the helpers
now live in the topical sibling gateway/run_turn_followup_ack.py.

Original commits (8828a3db37, f3c4f7a68c) by Mira Solari <268252643+mira-solari@users.noreply.github.com>:
--- 8828a3db37 fix(gateway): fire processing hooks for runner-drained queued follow-ups
A message that arrives while a turn is already running never gets the
processing-start acknowledgement — the 👀 read receipt on Slack, and the
equivalent in-progress reaction on Discord, Telegram, Feishu, Matrix,
Signal and Photon. It is not added-then-removed; the hook is never called.

`_run_processing_hook("on_processing_start", …)` has exactly one call site,
inside `BasePlatformAdapter._process_message_background` (base.py:5403).
`handle_message` takes the busy branch at base.py:5165, parks the event and
returns at base.py:5316 — above `_start_session_processing`, which is the
only thing that spawns `_process_message_background`. The parked event is
then drained in-band by the runner (`_dequeue_pending_event`, run.py:23277)
and replayed through a recursive `_run_agent` that touches no adapter hooks.
Because `get_pending_message` pops, the adapter's own drain (base.py:5823)
— the one path that would fire the hook — finds an empty slot. The gap is
structural, not a race, and it is shared by every mode (`queue`,
`interrupt`, steer-demoted-to-queue), by `/queue`, by photo-burst and
text-debounce flushes, and by voice drains.

Fire the existing hook pair around the recursive call. The follow-up now
gets the same lifecycle an idle-session message already gets, on every
platform, through one shared site.

Details that shaped the placement:

- Fired after the depth-cap requeue (run.py:23372) and after every discard
  and early return in the block, so no path can strand an in-progress
  marker: from that point on, control either reaches the recursion or
  raises, and both close the hook.
- Fired before `_refresh_agent_cache_message_count` so that re-baseline
  stays adjacent to the recursive call it exists to protect — inserting a
  reaction round-trip between them would widen the window in which the
  cross-process coherence guard (#45966) can trip on our own writes and
  rebuild the agent, destroying the prompt-cache prefix #46237 preserves.
- The hook adapter is resolved from the follow-up's own source, not the
  completing turn's: a multiplexed gateway can route it to a different
  profile's adapter, and only that instance holds the per-message reaction
  state.
- Gated on a truthy `message_id`, which is what every adapter's own hook
  already checks. Synthetic drains (`/goal` continuations, wake-ups, CLI
  hand-offs) carry no id and stay silent; `interrupt_message` and leftover
  `/steer` carry no event at all.
- Cancellation maps to CANCELLED rather than FAILURE, matching
  _process_message_background — Telegram clears the marker on CANCELLED and
  Signal deliberately leaves it, so the distinction is load-bearing.

Outcome is SUCCESS unless the recursion raises. That mirrors the existing
non-queued contract, where a run returning `failed: True` still delivers a
diagnostic message and reports SUCCESS; making the outcome track agent
failure is a separate change that should apply to both paths at once.

No new config, no new env var, no new hook, no change to message
construction or role alternation.

Fixes #72502

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

--- f3c4f7a68c fix(gateway): don't hand a completion to complete-only adapters; cover raw-envelope events
Self-review of the previous commit found three defects in it. All three are
about the *guard*, not the mechanism.

1. Complete-only adapters were handed an unpaired completion. Google Chat
   (google_chat/adapter.py:2766) and webhook (webhook.py:901) implement
   on_processing_complete WITHOUT on_processing_start, and theirs is
   end-of-cycle teardown, not a reaction: Google Chat reaps the typing card,
   patching it to "(no reply)"/"(interrupted)", and webhook ends the
   per-delivery session. The drain fires before the follow-up's reply is
   delivered — delivery happens after the whole chain unwinds back into
   _process_message_background — so a Google Chat space would get a permanent
   "(no reply)" tombstone on every queued follow-up, and the real answer would
   then land as a separate message. Now gated on the adapter actually
   overriding on_processing_start: we bracket, so both halves must be ours.

2. The message_id gate made the fix a no-op on Signal, which the previous
   commit message claimed to fix. SignalAdapter never sets message_id
   (signal.py:749-766) — its hook keys off raw_message["sender"] and
   ["timestamp_ms"] via _extract_reaction_target — and Discord's start hook
   reads raw_message too. Gate is now message_id OR raw_message; synthetic
   drains still carry neither, so /goal continuations stay silent.

3. Cancellation was classified unconditionally as CANCELLED. base.py:5862-5868
   only reports CANCELLED for a task in _expected_cancelled_tasks (/stop, /new,
   /reset, adapter cleanup) and downgrades anything else to FAILURE. Signal and
   Matrix deliberately LEAVE the marker in place on CANCELLED, so the previous
   version would strand exactly the marker this PR exists to clear. Now mirrors
   base.py via _followup_cancel_outcome().

Also moves _refresh_agent_cache_message_count inside the try. It awaits DB I/O
and guards it with `except Exception`, which does not catch cancellation, so a
/stop landing there escaped both handlers and stranded the marker. Ordering is
unchanged, so the prompt-cache adjacency the previous commit describes still
holds.

Two new tests, both failing against the previous commit:
test_complete_only_adapter_is_left_alone and
test_raw_envelope_only_followup_is_acknowledged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-13 20:52:02 -07:00

61 lines
3.1 KiB
Python

"""Processing-lifecycle hooks for runner-drained queued follow-ups.
A message that arrives mid-turn is parked in the adapter's pending slot and drained in-band by
``TurnRunner._run_agent_queued_followup``, never by ``BasePlatformAdapter._process_message_background``
— the only other call site for ``on_processing_start`` / ``on_processing_complete``. Without firing
them here every adapter that renders a read-receipt reaction from the hooks silently skips queued,
interrupting and steer-demoted messages (#72502, salvage #72503).
"""
from __future__ import annotations
import asyncio
from gateway.platforms.base import BasePlatformAdapter, MessageEvent, ProcessingOutcome
def _followup_processing_hooks_apply(adapter, event: MessageEvent | None) -> bool:
"""Both conditions are necessary: a real inbound platform message to acknowledge (adapters key their
marker off ``message_id`` — Slack/Telegram/Feishu/Matrix/Photon — or off ``raw_message`` — Signal,
Discord; synthetic drains such as ``/goal`` continuations, wake-ups and startup auto-resume carry
neither and must stay silent), and an adapter that overrides ``on_processing_start``. We bracket, so
both halves must belong to us: a complete-only adapter (Google Chat reaps its typing card there,
webhook ends its per-delivery session) would otherwise be handed a completion for a turn whose reply
is delivered only after the drain chain unwinds back into ``_process_message_background``."""
if adapter is None or event is None:
return False
if not (getattr(event, "message_id", None) or getattr(event, "raw_message", None)):
return False
start_hook = getattr(type(adapter), "on_processing_start", None)
return start_hook is not None and start_hook is not BasePlatformAdapter.on_processing_start
def _followup_cancel_outcome(adapter) -> ProcessingOutcome:
"""Classify a cancelled follow-up exactly as ``_process_message_background`` does: only cancels the
adapter itself routed (``/stop``, ``/new``, ``/reset``, cleanup) are CANCELLED, anything else is a
failure. Signal and Matrix leave the in-progress marker in place on CANCELLED, so reporting an
unexpected cancellation as CANCELLED would strand it."""
expected = getattr(adapter, "_expected_cancelled_tasks", None)
if expected is None:
return ProcessingOutcome.FAILURE
try:
current = asyncio.current_task()
except RuntimeError:
current = None
if current is None:
return ProcessingOutcome.FAILURE
try:
return ProcessingOutcome.CANCELLED if current in expected else ProcessingOutcome.FAILURE
except TypeError:
return ProcessingOutcome.FAILURE
async def _run_followup_processing_hook(adapter, event: MessageEvent | None, hook_name: str, *args) -> None:
"""Fire one lifecycle hook for a runner-drained follow-up; no-op per ``_followup_processing_hooks_apply``."""
if not _followup_processing_hooks_apply(adapter, event):
return
run_hook = getattr(adapter, "_run_processing_hook", None)
if not callable(run_hook):
return
await run_hook(hook_name, event, *args)