Skip to content

fix(agent): add return_exceptions=True to asyncio.gather in context reference expansion - #59954

Closed
x7peeps wants to merge 3 commits into
NousResearch:mainfrom
x7peeps:fix/context-ref-gather-return-exceptions
Closed

fix(agent): add return_exceptions=True to asyncio.gather in context reference expansion#59954
x7peeps wants to merge 3 commits into
NousResearch:mainfrom
x7peeps:fix/context-ref-gather-return-exceptions

Conversation

@x7peeps

@x7peeps x7peeps commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Added to in to prevent a single failing reference expansion from crashing the entire agent turn.

Problem

concurrently expands multiple context references via without . If any single call raises an exception (e.g. network failure for , permission error for ), the gather immediately propagates the exception, causing:

  1. All other references are silently abandoned — even ones that would have succeeded
  2. The entire agent turn crashes instead of gracefully degrading
  3. The in catches most errors, but unexpected errors (e.g. , subclasses) still propagate

Fix

  • Added to the call
  • Iterate results and convert items into warning messages
  • Other reference expansions continue unaffected when one fails

Testing

  • Syntax check passed (py_compile)
  • Behavior verified: exceptions from individual references now surface as warnings instead of crashing the turn

@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 the context-reference gather path. Current main still has the unguarded gather at agent/context_references.py:161, so the focused hunk remains relevant.

Problems

  • agent/moa_loop.py:384 only times out Future.result(). The surrounding ThreadPoolExecutor context (agent/moa_loop.py:407) still owns unfinished workers at scope exit, so a hung reference can still block return.
  • tools/voice_mode.py:1109 can raise a second TimeoutExpired inside the first timeout handler. That skips the cleanup at lines 1110-1111 and can leave _active_playback set.
  • No test file is included. Add a context-reference regression with one escaping child failure and one successful sibling; the existing concurrency tests only cover successful fetches.

Suggested changes

  • Keep this PR focused on agent/context_references.py (or split the independent MoA and voice fixes), then add the regression test.
  • Rework the MoA and voice timeout handling with explicit cleanup and tests before proposing them separately.

Automated hermes-sweeper review.

Comment thread agent/moa_loop.py
for future, idx in futures.items():
results[idx] = future.result()
try:
results[idx] = future.result(timeout=300)

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 does not bound the time until _run_references_parallel returns: timing out future.result() leaves the worker running, and the surrounding executor context still waits as it exits. Use a design that can return without waiting for timed-out workers, or split this unrelated change.

Comment thread tools/voice_mode.py
logger.warning("System player %s timed out, killing process", cmd[0])
proc.kill()
proc.wait()
proc.wait(timeout=10)

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.

proc.wait(timeout=10) can itself raise TimeoutExpired inside this handler; the following except Exception will not catch it, and _active_playback is not cleared. Handle the second timeout and perform cleanup in finally.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state 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 15, 2026
@x7peeps x7peeps closed this Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have 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-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state tool/tts Text-to-speech and transcription type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants