fix: surface OpenAI chat-completions refusal as assistant text#102344
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 8, 2026, 10:58 PM ET / 02:58 UTC. Summary PR surface: Source +26, Tests +150. Total +176 across 4 files. Reproducibility: yes. Source inspection of current main gives a high-confidence path: feed either completions assembler a refusal-only delta or message chunk and current main never reads the refusal field. Review metrics: none identified. Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. 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 detailsBest possible solution: Land one canonical branch that handles refusal in both completions assemblers with delta and message-form coverage, then close the linked issue and duplicate PRs after merge. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection of current main gives a high-confidence path: feed either completions assembler a refusal-only delta or message chunk and current main never reads the refusal field. Is this the best way to solve the issue? Yes. The PR takes the narrow maintainable path: parser parity in both chat-completions assemblers, with separate refusal fallback/error policy left to other work. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 30b3a7cdeda9. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +26, Tests +150. Total +176 across 4 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)
|
a250cb0 to
db6db43
Compare
|
Added real-behavior proof beyond the mock chunk tests.
The runtime proof used a real local HTTP SSE response and the actual OpenAI SDK request/parse path, so it covers more than the in-memory iterable mocks. |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Land-ready maintainer review complete on exact head The linked refusal-only message-loss bug reproduces from current source in both active chat-completions assemblers. This patch fixes the parsing owners directly and keeps refusal fallback/error policy unchanged. Proof:
Best-fix judgment: yes. Both active assemblers now match the existing Responses refusal-to-visible-text behavior, without adding fallback policy or moving parsing into a later projection. |
|
Merged via squash.
|
…law#102344) * fix: surface OpenAI chat-completions refusal as assistant text * fix: cover message.refusal on packages/ai completions path
* fix: surface OpenAI chat-completions refusal as assistant text * fix: cover message.refusal on packages/ai completions path (cherry picked from commit 3fd5251)
Fixes #102321
What Problem This Solves
Fixes an issue where users on openai-completions models would receive a blank assistant turn when the provider returned a safety or structured-output refusal with
content: nulland a top-levelrefusalfield.Why This Change Was Made
Chat Completions can put the refusal text only in
choice.delta.refusal/message.refusal. The completions stream assemblers already handledcontent, reasoning, and tool calls, but never readrefusal, so the turn resolved to empty content. The Responses path already surfaces refusal text; this change brings completions parity in both the managed agent transport and the packages/ai provider assembler.Follow-up on ClawSweeper review: packages/ai now normalizes
choice.delta ?? choice.messagethe same way the agent transport does, so aggregated message-only refusal chunks are covered, with matching regression tests. The agent transport message.refusal test cast was also fixed forcheck:test-types.User Impact
When a model refuses on the chat-completions API, users now see the refusal text instead of an empty reply.
Evidence
Focused stream-assembler regressions (agent + packages/ai, delta and message forms):
packages/ai verbose (post-review message.refusal path):
Also: oxfmt clean;
pnpm check:test-typespath previously failed on the agent message cast — fixed viaas unknown as Choice.Real behavior proof
refusalfield (content null), delivered as eitherdelta.refusalor aggregatedmessage.refusal, no longer resolve to empty content; refusal text is shown as normal assistant text on both the managed agent transport and packages/ai streamOpenAICompletions assembler.origin/main; production stream functions exercised via exported/test entrypoints (processOpenAICompletionsStream,streamOpenAICompletionswith SDK create mocked to yield real chunk shapes).refusal(and packages/ai only entered the content path forchoice.delta), so a chunk withcontent: null+refusal: "..."producedcontent: []. After the patch, the same payloads produce a visible text block with the refusal string on both assemblers.deltachunk into agentprocessOpenAICompletionsStream.messagechunk (nodelta) into the same function.streamOpenAICompletions(mocked OpenAI stream).contentis[{ type: "text", text: "<refusal>" }]and stop reason remainsstop.surfaces aggregated chat-completions message.refusal as visible assistant text.api.openai.comnetwork call that returns a real safety refusal (noOPENAI_API_KEYin this environment). Protocol-level behavior is covered by production assembler code paths with provider-shaped chunks.AI disclosure
This fix was authored with AI assistance (Grok). Implementation and tests follow the ClawSweeper-recommended narrow parser-parity approach, including the packages/ai
message.refusalcompleteness fix from review.