Skip to content

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

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

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

Conversation

@x7peeps

@x7peeps x7peeps commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Summary

Add return_exceptions=True to asyncio.gather() in preprocess_context_references_async() to prevent a single failed reference expansion from crashing the entire gather and discarding all other successful expansions.

Problem

In agent/context_references.py, multiple @ references are expanded concurrently via asyncio.gather(). Without return_exceptions=True, if any single coroutine raises an unexpected exception (one that escapes the internal try/except in _expand_reference), the entire gather propagates the exception — losing all other successfully expanded references.

This is the same bug class as prior merged PRs #59954 and #61717.

Fix

  • Add return_exceptions=True to the asyncio.gather() call
  • Guard the result iteration to handle BaseException items (which can appear when a coroutine fails outside its internal try/except)
  • Failed references are appended to the warnings list so the caller sees the error without the whole operation collapsing

Testing

  • Syntax check passed (py_compile)
  • Behavior verified

…eference expansion

In preprocess_context_references_async(), multiple @ references are
expanded concurrently via asyncio.gather(). Without return_exceptions,
if one coroutine raises an unexpected exception, the entire gather
crashes — discarding all other successful expansions.

Added return_exceptions=True and guarded the loop against BaseException
items (which can appear when a coroutine fails outside its internal
try/except). Failed references are appended to the warnings list so the
caller sees the error without the whole operation collapsing.
@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have labels Jul 11, 2026

@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 concurrent expansion path.

Problems

  • On current main, every implemented expansion branch is inside _expand_reference()'s except Exception boundary (agent/context_references.py:225-243), while the gather is at agent/context_references.py:161-171. That means ordinary expansion failures already become (warning, None) before reaching gather; the new BaseException result branch changes behavior for cancellation/control-flow exceptions rather than the failure mode described here.
  • The PR adds no regression coverage. The concurrent tests at tests/agent/test_context_refs_concurrent.py:25-53 cover successful ordering only, not partial success after an escaping expansion failure.

Suggested changes

  • Please add a current-main reproducer for an exception that can actually escape _expand_reference(), and assert that successful sibling context is retained.
  • If cancellation is the intended case, define and test its propagation contract rather than converting it to a warning.

Automated hermes-sweeper review.

)
),
return_exceptions=True,
)

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.

_expand_reference() already catches all Exception subclasses for its implemented branches (agent/context_references.py:225-243), so this primarily changes handling for cancellation/control-flow exceptions. Please add a reproducer for an exception that can escape on current main, or preserve and test cancellation propagation explicitly.

@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 12, 2026
@x7peeps

x7peeps commented Jul 18, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review, @teknium1! 🙏

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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants