fix: Responses streams mishandle interleaved output items and silent truncation#109615
Merged
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5de387f772
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+995
to
+996
| output.stopReason = "error"; | ||
| output.errorMessage = "Provider incomplete_reason: content_filter"; |
Contributor
There was a problem hiding this comment.
When the ChatGPT Responses transport receives response.incomplete with incomplete_details.reason === "content_filter", this new branch makes processResponsesStream return normally with output.stopReason = "error". The ChatGPT caller does not use runResponsesStreamLifecycle; after processStream/processWebSocketStream it unconditionally pushes a done event, so SSE/WebSocket ChatGPT content-filter failures are surfaced as completed turns to consumers that branch on terminal event type. Add the same post-process error check used by the OpenAI/Azure lifecycle before emitting done.
Useful? React with 👍 / 👎.
steipete
force-pushed
the
fix/responses-stream-hardening
branch
from
July 17, 2026 03:56
5de387f to
82b807c
Compare
clawsweeper
Bot
added
rating: 🦐 gold shrimp
Contributor
|
Codex review: found issues before merge. Reviewed July 17, 2026, 1:50 AM ET / 05:50 UTC. Summary PR surface: Source +228, Tests +740. Total +968 across 11 files. Reproducibility: yes. at the source-contract level: current main's single-slot parsing and clean completion behavior are directly traceable, and the PR includes focused regression cases plus live OpenAI after-fix proof. This review did not independently execute a failing current-main live stream. Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review findings
Review detailsBest possible solution: Centralize terminal stream finalization so OpenAI, Azure, and ChatGPT callers all emit an error event for Do we have a high-confidence way to reproduce the issue? Yes at the source-contract level: current main's single-slot parsing and clean completion behavior are directly traceable, and the PR includes focused regression cases plus live OpenAI after-fix proof. This review did not independently execute a failing current-main live stream. Is this the best way to solve the issue? No, not yet. Per-output-index state and explicit terminal enforcement are the right layer, but terminal finalization must be shared or consistently applied by the ChatGPT SSE/WebSocket callers before this is the best fix. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against f977649065cd. Label changesLabel justifications:
Evidence reviewedPR surface: Source +228, Tests +740. Total +968 across 11 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (3 earlier review cycles)
|
clawsweeper
Bot
added
rating: 🦪 silver shellfish
clawsweeper
Bot
added
proof: sufficient
Contributor
Author
|
Merged via squash.
|
This was referenced Jul 17, 2026
github-actions Bot
pushed a commit
to Desicool/openclaw
that referenced
this pull request
Jul 18, 2026
…truncation (openclaw#109615) * fix(ai): track Responses output items per index and require terminal stream events * test(ai): add live Responses stream coverage * test(infra): mirror packages live glob in live-config test
steipete
added a commit
that referenced
this pull request
Jul 18, 2026
…nd zero cost (#109904) * fix(agents): record token usage when a Responses turn ends incomplete The agent-side Responses processor only had a terminal branch for response.completed. A stream that ends with response.incomplete — the max_output_tokens and content_filter cases, and what Azure emits on early truncation — matched no branch at all, so the event was dropped: usage was never recorded and stopReason was never set. The turn reports zero tokens and zero cost, which is the drift in #100954. #109615 fixed the same split on the package-side processor by finalizing completed and incomplete through one finalizeResponse. This does the same for the agent path, which #109615 did not touch: both terminal events now record usage, cost and service-tier pricing through one helper. The helper moves to its own module rather than growing openai-responses-transport.ts, which is a legacy file the max-lines ratchet will not let grow; extracting the block drops it from 2502 to 2444 lines. Content-filtered turns are mapped to a provider error instead of a plain length stop, matching what the package side already does, so the two terminal surfaces do not disagree. * fix(agents): keep Responses output backfill on completed turns Terminal handling now covers response.incomplete, which also routed those events into backfillCompletedResponseOutput. That reconstruction exists to recover a final answer when item events never arrived; an incomplete turn has no final answer, so replaying its partial output persisted truncated text the streaming path never emitted. Usage and stop-reason recording still apply to both terminal events. * refactor(ai): share one Responses terminal usage mapper The agent transport mapped terminal usage buckets, cost, and stop reasons in parallel with the package-side processor, so the two could drift on token buckets, service-tier pricing, or future terminal-event semantics. Both now call one canonical mapper. openai-responses-shared.ts cannot be imported across the package boundary, so the mapper lives in its own module re-exported through the existing internal/openai entry point; no new subpath is introduced. The agent module keeps only the reasoning-token accounting the package path does not track. Merging the two revealed a real disagreement on totalTokens: the package took the reported total, the agent summed the split buckets. The canonical rule is now max(bucket sum, reported total), which keeps the reported value while covering both payloads that omit total_tokens and payloads whose cached_tokens exceed input_tokens, where clamping leaves the reported total short. * fix(agents): preserve partial incomplete Responses output Co-authored-by: Yigtwxx <[email protected]> * test(agents): cover incomplete terminal output backfill boundaries The head's partial-output preservation is only safe while it stays a recovery path. Pin both sides of that guard: text that already streamed must not be replayed from the terminal payload, and a non-length incomplete stop must not surface partial text as an answer. * fix(agents): avoid shadowing Responses options Co-authored-by: Yigtwxx <[email protected]> * style(agents): apply oxfmt to the incomplete backfill test * test(agents): register incomplete Responses suite Co-authored-by: Yiğit ERDOĞAN <[email protected]> --------- Co-authored-by: Peter Steinberger <[email protected]>
github-actions Bot
pushed a commit
to Desicool/openclaw
that referenced
this pull request
Jul 19, 2026
…nd zero cost (openclaw#109904) * fix(agents): record token usage when a Responses turn ends incomplete The agent-side Responses processor only had a terminal branch for response.completed. A stream that ends with response.incomplete — the max_output_tokens and content_filter cases, and what Azure emits on early truncation — matched no branch at all, so the event was dropped: usage was never recorded and stopReason was never set. The turn reports zero tokens and zero cost, which is the drift in openclaw#100954. openclaw#109615 fixed the same split on the package-side processor by finalizing completed and incomplete through one finalizeResponse. This does the same for the agent path, which openclaw#109615 did not touch: both terminal events now record usage, cost and service-tier pricing through one helper. The helper moves to its own module rather than growing openai-responses-transport.ts, which is a legacy file the max-lines ratchet will not let grow; extracting the block drops it from 2502 to 2444 lines. Content-filtered turns are mapped to a provider error instead of a plain length stop, matching what the package side already does, so the two terminal surfaces do not disagree. * fix(agents): keep Responses output backfill on completed turns Terminal handling now covers response.incomplete, which also routed those events into backfillCompletedResponseOutput. That reconstruction exists to recover a final answer when item events never arrived; an incomplete turn has no final answer, so replaying its partial output persisted truncated text the streaming path never emitted. Usage and stop-reason recording still apply to both terminal events. * refactor(ai): share one Responses terminal usage mapper The agent transport mapped terminal usage buckets, cost, and stop reasons in parallel with the package-side processor, so the two could drift on token buckets, service-tier pricing, or future terminal-event semantics. Both now call one canonical mapper. openai-responses-shared.ts cannot be imported across the package boundary, so the mapper lives in its own module re-exported through the existing internal/openai entry point; no new subpath is introduced. The agent module keeps only the reasoning-token accounting the package path does not track. Merging the two revealed a real disagreement on totalTokens: the package took the reported total, the agent summed the split buckets. The canonical rule is now max(bucket sum, reported total), which keeps the reported value while covering both payloads that omit total_tokens and payloads whose cached_tokens exceed input_tokens, where clamping leaves the reported total short. * fix(agents): preserve partial incomplete Responses output Co-authored-by: Yigtwxx <[email protected]> * test(agents): cover incomplete terminal output backfill boundaries The head's partial-output preservation is only safe while it stays a recovery path. Pin both sides of that guard: text that already streamed must not be replayed from the terminal payload, and a non-length incomplete stop must not surface partial text as an answer. * fix(agents): avoid shadowing Responses options Co-authored-by: Yigtwxx <[email protected]> * style(agents): apply oxfmt to the incomplete backfill test * test(agents): register incomplete Responses suite Co-authored-by: Yiğit ERDOĞAN <[email protected]> --------- Co-authored-by: Peter Steinberger <[email protected]>
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.
What Problem This Solves
Fixes a set of OpenAI Responses-protocol streaming defects that corrupt reasoning replay and hide stream truncation:
output_indexes) lose reasoning deltas or attach a thinking signature to the wrong block, breaking multi-turn reasoning replay.response.incompletewas not handled at all.encrypted_contentonly arrives in the terminalresponse.completedpayload (Azure behavior) cannot be replayed statelessly.output_item.donewithcontent: null(some OpenAI-compatible servers) crashes the stream mid-flight.max_output_tokensbelow 16 is rejected by the API with a 400.developerrole could not run reasoning models.store: falsewas only set on the plain OpenAI path).Why This Change Was Made
The single-slot stream state machine assumed strictly sequential output items, which the Responses protocol does not guarantee. Tracking per-
output_indexslots makes interleaving safe, and requiring an explicit terminal event turns silent truncation into a classified, retryable error.User Impact
developer-role support; Azure gainsstore: falseprivacy parity; retry behavior is fully owned by the app's policy.Evidence
node scripts/run-vitest.mjs packages/ai/src/providers/openai-responses-shared.test.ts packages/ai/src/providers/openai-responses.test.ts packages/ai/src/providers/azure-openai-responses.test.ts packages/ai/src/providers/openai-chatgpt-responses.test.ts— 4 files, 113 tests passing, including new cases for interleaved reasoning by output index, deferred text across interleaved items, terminal encrypted-content backfill,response.incompletewith usage, content-filter classification, missing-terminal-event error, header clamping, and retry-budget honoring.node scripts/check-changed.mjs).Live verification (real API keys):
node scripts/test-live.mjs -- packages/ai/src/providers/openai-responses.live.test.ts— 3/3 passing against the live OpenAI Responses API: terminal-event usage accounting, sub-floormax_output_tokensclamp accepted (previously a 400), and an interleaved reasoning + tool-call stream keeping items separable with intact thinking signatures. Live suite added in this PR (gated onOPENCLAW_LIVE_TEST=1+ key).