fix(gateway): close silent response loss after agent tool calls (#29346)#34336
Merged
teknium1 merged 2 commits intoJun 2, 2026
Merged
Conversation
e467c0b to
f40bfea
Compare
…esearch#29346) A streamed preamble ("Let me search...") finalized at a tool boundary routed through _try_fresh_final, which unconditionally set _final_response_sent=True even though it is a NON-final segment. The gateway then reads that flag as "final delivered" and suppresses the genuine final answer produced on the next API call, so the user silently gets nothing. Only reproduces with fresh_final_after_seconds > 0. - _try_fresh_final / _send_or_edit take is_turn_final; the segment-break call site passes is_turn_final=got_done so only the turn-final answer marks final-delivered. - _reset_segment_state clears the final-delivery flags at every tool boundary as defense-in-depth against any future premature setter. - Failing-first regression + happy-path no-duplicate test.
NousResearch#29346) The extract pipeline (extract_media/extract_images/extract_local_files + directive strips) can reduce a non-empty tool-using response to empty text_content with no deliverable attachment. The 'if text_content' send guard then silently skips delivery: a 'response ready' log with no 'Sending response', no error, and the answer never reaches the user. - A2: snapshot the pre-extract response; when extraction yields empty text and no image/local/media attachment, deliver the recovered original from the post-extract_media body (so a spaced MEDIA path can't leak). Applies on ALL platforms (supersedes the Discord-only NousResearch#33842 and the unsafe raw-fallback NousResearch#29499). - A3: loud delivery invariant - a non-empty response that produces nothing deliverable logs response_delivery_dropped at ERROR; every recovery logs response_delivery_recovered. No silent drop survives. - Factor a _strip_media_directives helper for the [[...]] strips; MEDIA stripping stays owned by extract_media, whose grammar handles spaced and quoted paths. - Salvaged + de-scoped the NousResearch#33842 test harness to all platforms; added unrecoverable-drop and no-leak regression tests.
f40bfea to
8bee4ea
Compare
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On gateways, a tool-using turn could produce a correct, non-empty final answer
that the user never received — logs show
response readywith no matchingSending responseand no error. This fixes the two independent gateway-sidecauses and adds a loud invariant so a non-empty response is no longer dropped
silently.
Root cause
turn as already-delivered, so the gateway suppressed the genuine answer
produced on the next API call. (Only with
fresh_final_after_seconds > 0.)can reduce a non-empty response to empty text with no attachment; the
if text_contentsend guard then skips delivery silently.Fix
clear them at every tool boundary, so a preamble can no longer mark the turn
delivered.
original — taken from the post-extract body so a spaced
MEDIA:path can'tleak — on every platform.
response_delivery_recovered(WARNING); any genuinely undeliverable non-empty response logs
response_delivery_dropped(ERROR).Closes / supersedes
Closes #29346. Supersedes #33842 (Discord-only) and #29499 by consolidating both
into one all-platform fix that sanitizes the recovered text and won't re-send
alongside an attachment; #33842's test harness is salvaged and generalized.
Testing
Failing-first regressions for both causes; an exactly-once test guarding against
duplicate sends; a loud-drop ERROR test; and a no-leak test for spaced
MEDIA:paths. The gateway delivery and stream-consumer suites pass.
Risk
Scoping the final-delivery state to the turn-final segment is duplicate-safe by
construction —
got_donereturns before any reset, and the gateway reads thatstate only after the stream task exits. The recovery and invariant are additive
(recover-or-log only, no change to existing send behavior). Known limitation: the
invariant treats a produced attachment as "delivered" even if its send later
fails, but those failures are already logged at the send site.
Out of scope
The freeze in #28834 (tool calls hanging after idle on macOS) is a separate
CLI/event-loop bug, not gateway delivery loss — not addressed here.
Infographic