fix: propagate Reactor context to chunk event hooks in ReActAgent#1823
fix: propagate Reactor context to chunk event hooks in ReActAgent#1823chcodex wants to merge 2 commits into
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
AgentScopeJavaBot
left a comment
AgentScopeJavaBot
left a comment
There was a problem hiding this comment.
🤖 AI Review
This PR fixes a real Reactor context propagation bug where three fire-and-forget .subscribe() calls in ReActAgent (fireReasoningChunk, fireActingChunk, fireSummaryChunk) created detached subscription chains that lost the parent Reactor context. The fix correctly applies Flux.deferContextual + contextWrite to capture and propagate the parent context, matching the existing pattern already used by executeToolCalls in runToolBatch. The changes are minimal, focused, and consistent with the codebase conventions. No correctness issues found.
| mci.model().getModelName()) | ||
| .subscribe(); | ||
| } | ||
| return Flux.deferContextual( |
There was a problem hiding this comment.
[praise] Clean fix. Wrapping the concatMap body in Flux.deferContextual correctly captures the parent Reactor context at subscription time, ensuring fireReasoningChunk hooks see the full ContextView (including SubagentEventBus). The pattern is consistent with the existing executeToolCalls context propagation.
| hookDispatcher | ||
| .fireActingChunk( | ||
| toolUse, chunk, toolkit) | ||
| .contextWrite( |
There was a problem hiding this comment.
[praise] Good catch — the fireActingChunk call was the most subtle of the three since parentCtx was already in scope from the enclosing Flux.deferContextual. Adding just .contextWrite(ctx -> ctx.putAll(parentCtx)) here is the minimal correct change.
| model.getModelName()) | ||
| .subscribe(); | ||
| } | ||
| return Flux.deferContextual( |
There was a problem hiding this comment.
[praise] Symmetric fix to modelCallStream. The Flux.deferContextual + contextWrite pattern is applied consistently, ensuring fireSummaryChunk hooks also receive the parent context.
AgentScopeJavaBot
left a comment
AgentScopeJavaBot
left a comment
There was a problem hiding this comment.
🤖 AI Review
This PR fixes a real Reactor context propagation bug where three fire-and-forget .subscribe() calls in ReActAgent (fireReasoningChunk, fireActingChunk, fireSummaryChunk) created detached subscription chains that lost the parent Reactor context. The fix correctly applies Flux.deferContextual + contextWrite to capture and propagate the parent context, matching the existing pattern already used by executeToolCalls in runToolBatch. The changes are minimal, focused, and consistent with the codebase conventions. No correctness issues found.
| mci.model().getModelName()) | ||
| .subscribe(); | ||
| } | ||
| return Flux.deferContextual( |
There was a problem hiding this comment.
[praise] Clean fix. Wrapping the concatMap body in Flux.deferContextual correctly captures the parent Reactor context at subscription time, ensuring fireReasoningChunk hooks see the full ContextView (including SubagentEventBus). The pattern is consistent with the existing executeToolCalls context propagation.
| hookDispatcher | ||
| .fireActingChunk( | ||
| toolUse, chunk, toolkit) | ||
| .contextWrite( |
There was a problem hiding this comment.
[praise] Good catch — the fireActingChunk call was the most subtle of the three since parentCtx was already in scope from the enclosing Flux.deferContextual. Adding just .contextWrite(ctx -> ctx.putAll(parentCtx)) here is the minimal correct change.
| model.getModelName()) | ||
| .subscribe(); | ||
| } | ||
| return Flux.deferContextual( |
There was a problem hiding this comment.
[praise] Symmetric fix to modelCallStream. The Flux.deferContextual + contextWrite pattern is applied consistently, ensuring fireSummaryChunk hooks also receive the parent context.
AgentScopeJavaBot
added
bug
|
@chickenlj Hello, is there any problem with this PR? I saw that it has not been merged for a long time. |
Thanks for the contribution. There're conflicts caused by other pull requests and I have fixed this issue with another request. |
3 participants
fix: propagate Reactor context to chunk event hooks in ReActAgent
Three bare .subscribe() calls in ReActAgent (fireReasoningChunk,
fireActingChunk, fireSummaryChunk) created new subscription chains
without the parent Reactor context, causing ReasoningChunkEvent,
ActingChunkEvent, and SummaryChunkEvent to have an empty ContextView
in downstream hooks.
Fix each with Flux.deferContextual + contextWrite to capture and
propagate the parent context, matching the existing pattern used by
executeToolCalls at line 2389-2391.