Skip to content

fix: coalesce multiple background process completion notifications by session_key - #70319

Closed
handnewb wants to merge 2 commits into
NousResearch:mainfrom
handnewb:fix/coalesce-background-process-notifications
Closed

fix: coalesce multiple background process completion notifications by session_key#70319
handnewb wants to merge 2 commits into
NousResearch:mainfrom
handnewb:fix/coalesce-background-process-notifications

Conversation

@handnewb

Copy link
Copy Markdown
Contributor

Problem

When multiple background processes finish in the same gateway tick, each one injects a separate notification into the agent session via _inject_watch_notification. This floods the context with redundant messages, causes session lag (Persisted transcript lagged), and leaves the agent processing notification-after-notification instead of responding to the user.

Evidence from production (Discord gateway)

17:24:53.057 Process proc_99a794b9b5e9 finished — injecting agent notification
17:24:53.060 Process proc_ce2c0b31a186 finished — injecting agent notification
17:24:53.063 Process proc_12944841da9c finished — injecting agent notification
17:24:53.067 Process proc_7833aa6465b5 finished — injecting agent notification
17:24:53.070 Process proc_0f220e4e1655 finished — injecting agent notification
17:24:53.074 Process proc_d2824952ce08 finished — injecting agent notification

Then immediately:

WARNING Persisted transcript lagged live cached history (disk=154, memory=157)

6 processes → 6 injections → flooded session → unresponsive agent.

Fix

Added GatewayRunner._coalesce_and_inject_watch_events() which groups standard completion events by session_key before injection. When N processes complete for the same session in one drain tick, a single batched message is delivered:

[IMPORTANT: 6 background processes finished — results batched to avoid session flood. Use process(id=N, action='log') to inspect individual outputs.]

Key design decisions:

  • Only standard completions are coalescedwatch_match events pass through individually since each pattern match is independently important
  • Grouping by session_key, not globally — completions for different sessions remain separate
  • Single-event pass-through is unchanged — zero overhead when only one process finishes

Changes

  • gateway/run.py:

    • New method _coalesce_and_inject_watch_events() with coalescing logic
    • Updated post-turn drain loop to use new coalesced method
    • Extended _format_gateway_process_notification() to handle standard completion event type (previously returned None)
  • tests/gateway/test_background_process_notifications.py:

    • 7 new tests covering: single event passthrough, coalescing same session_key, separate session_keys, watch_match non-coalescing, mixed event types, empty list noop

Testing

41 passed in 5.18s (all existing + 7 new)

Closes #70300

… session_key

When multiple background processes finish in the same gateway tick,
each one injected a separate notification into the agent session,
flooding the context and causing session lag.

This change adds _coalesce_and_inject_watch_events() which groups
standard completion events by session_key before injection. When N
processes complete for the same session, a single batched message
like '[IMPORTANT: N background processes finished]' is delivered
instead of N individual messages.

watch_match events are NOT coalesced — each pattern match is
individually important and must reach the agent separately.

Closes NousResearch#70300
@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery tool/terminal Terminal execution and process management P2 Medium — degraded but workaround exists sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jul 23, 2026
…debug log, test type=None

- Batched message now includes process session_ids (truncated to 5 + '...and N more')
- Added logger.debug when coalescing events
- New test: test_coalesced_message_truncates_ids_after_5
- New test: test_type_none_treated_as_completion
- Updated existing test to verify IDs in coalesced message
@yuzilongleif-collab

Copy link
Copy Markdown
Contributor

I reproduced #70300 against current main and traced standard completions through the production call path.

One important ownership detail: standard completion events are delivered directly by each _run_process_watcher() through _deliver_completion_notification(). _drain_gateway_watch_events() returns watch_match events but does not return standard completions, so _coalesce_and_inject_watch_events() is not reached for the reported production path. The current unit tests call that helper directly and therefore do not detect the disconnect.

I opened #71898 with batching at the _run_process_watcher() delivery seam plus an end-to-end test that starts three concurrent watchers and asserts one adapter injection. It also preserves per-process retry/dedupe bookkeeping and covers route isolation, in-flight flush races, duplicate-primary handling, and formatter failure.

Sharing this here to connect the two approaches and avoid duplicated debugging; maintainers can choose whichever shape best fits the gateway ownership model.

@handnewb

handnewb commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

@yuzilongleif-collab — great catch on the architectural issue. You're absolutely right that standard completions flow through _run_process_watcher()_deliver_completion_notification() and never reach the post-turn drain where my original _coalesce_and_inject_watch_events() sat.

I've opened a new approach in #72675 that:

  1. Bathes standard completions at _run_process_watcher() (the correct seam, following your insight) with a Future-based pattern similar to fix(gateway): coalesce concurrent process completions #71898
  2. PLUS coalesces watch_match/watch_disabled events at the post-turn drain — which neither PR covered
  3. Adds threshold-based early flush (5+ entries → immediate), visual status indicators (✅/❌), aggregate summary, elapsed time, and zero-latency single completions

89 tests passing. Closing this in favor of the combined approach. Thanks for the thorough review!

@handnewb handnewb closed this Jul 27, 2026
handnewb pushed a commit to handnewb/hermes-agent that referenced this pull request Jul 28, 2026
Coalesce multiple background process completions and watch events that
share the same gateway route so the agent receives one synthetic turn
instead of one turn per process (NousResearch#70300).

Two-pronged approach, each at the correct ownership seam:

1. Standard completions: batched at _run_process_watcher() via
   _enqueue_process_completion_notification() with a short (100ms)
   window and threshold-based early flush (5+ entries = immediate).
   Single completions pass through with zero extra latency.

2. watch_match / watch_disabled events: coalesced at the post-turn
   drain via _coalesce_and_inject_watch_events(), grouped by type
   and session_key.

Output format includes:
- Per-process status with visual indicators (✅/❌)
- Exit codes, elapsed time, reason
- Aggregate summary (N succeeded, M failed)
- Bounded output (10 detailed results, 800-char tails)

Edge cases covered:
- Flush during delivery schedules next batch
- Duplicate primary tries next batch identity
- Formatter failure resolves all waiters with False
- Lazy init for tests using object.__new__
- None-safe batch key and type handling

Closes NousResearch#70300
Supersedes NousResearch#70319 and NousResearch#71898

Co-authored-by: yuzilongleif-collab <235949691+yuzilongleif-collab@users.noreply.github.com>
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:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages tool/terminal Terminal execution and process management type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Multiple background process completions in same tick flood session with individual notifications instead of coalescing

3 participants