fix(streaming-overflow): preserve hadVisibleText state when escalating to recovery (v0.5.39)#144
Merged
Merged
Conversation
…g to gateway recovery (Codex P2) When a text_delta is emitted before a model_error overflow event, the hadVisibleText flag was lost because handleStreaming() threw before completing normally. The gateway catch then always saw hadVisibleText=false and posted 'Please resend your last message' copy — misleading after partial output. Attach hadVisibleText to the StreamModelOverflowError so it survives the throw. Extract it in gateway.ts catch and pass to recovery, which then posts 'Response was interrupted; session recovered' wording for partial output (correct UX, prevents duplicate side effects). Test: updated streaming-overflow.test.ts to extract state from error object instead of relying on gateway hoisted-local pattern (which was the bug). All 591 tests green.
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
1 participant
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.
Issue
Codex-identified P2 regression in v0.5.38: when a streaming
text_deltaevent precedes amodel_erroroverflow, thehadVisibleTextflag is lost.Root cause:
handleStreamingthrowsStreamModelOverflowErroron overflow, bypassing the normal return path. The gateway's hoisted-localstreamHadVisibleTextis only updated on success, so it staysfalseeven though partial text was emitted.UX impact: Recovery posts 'Please resend your last message' instead of 'Response was interrupted; session recovered'. After partial output, this misleads the user and encourages duplicate sends (re-executing tools).
Fix
Attach
hadVisibleTextto theStreamModelOverflowErrorobject so it survives the throw. Extract it in gateway.ts catch via type-safeinstanceofand pass to recovery helper.Changes:
StreamModelOverflowErrorconstructor: addhadVisibleText: booleanparamstreaming.tsmodel_errorhandler: passhasVisibleTextwhen throwinggateway.tscatch: extracthadVisibleTextfrom error viainstanceofcheck, pass to recoverystreaming-overflow.test.ts: fix test to extract state from error (was manually overriding to true, masking the bug)Tests: 591 passing, no new failures.
Codex reviews: ✅ code review clean, ✅ arch review (type-safe instanceof) applied.
Closes Codex P2 feedback on PR #143.