Skip to content

fix: surface OpenAI chat-completions refusal as assistant text#102344

Merged
vincentkoc merged 3 commits into
openclaw:mainfrom
wuqxuan:grok/openai-completions-refusal-102321
Jul 9, 2026
Merged

fix: surface OpenAI chat-completions refusal as assistant text#102344
vincentkoc merged 3 commits into
openclaw:mainfrom
wuqxuan:grok/openai-completions-refusal-102321

Conversation

@wuqxuan

@wuqxuan wuqxuan commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

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: null and a top-level refusal field.

Why This Change Was Made

Chat Completions can put the refusal text only in choice.delta.refusal / message.refusal. The completions stream assemblers already handled content, reasoning, and tool calls, but never read refusal, 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.message the 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 for check: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):

node scripts/run-vitest.mjs \
  src/agents/openai-transport-stream.test.ts \
  packages/ai/src/providers/openai-completions.test.ts \
  -t "refusal" --maxWorkers=2
# 6 passed

packages/ai verbose (post-review message.refusal path):

✓ surfaces chat-completions refusal deltas as visible assistant text
✓ surfaces aggregated chat-completions message.refusal as visible assistant text

Also: oxfmt clean; pnpm check:test-types path previously failed on the agent message cast — fixed via as unknown as Choice.

Real behavior proof

  • Behavior addressed: openai-completions assistant turns that carry only a top-level refusal field (content null), delivered as either delta.refusal or aggregated message.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.
  • Environment: local macOS Node/Vitest against PR branch rebased on current origin/main; production stream functions exercised via exported/test entrypoints (processOpenAICompletionsStream, streamOpenAICompletions with SDK create mocked to yield real chunk shapes).
  • Current-main contrast: before the patch, both assemblers ignored refusal (and packages/ai only entered the content path for choice.delta), so a chunk with content: null + refusal: "..." produced content: []. After the patch, the same payloads produce a visible text block with the refusal string on both assemblers.
  • Exact steps or command run after this patch:
    1. Feed a refusal-only delta chunk into agent processOpenAICompletionsStream.
    2. Feed a refusal-only aggregated message chunk (no delta) into the same function.
    3. Feed the same shapes through packages/ai streamOpenAICompletions (mocked OpenAI stream).
    4. Assert assistant content is [{ type: "text", text: "<refusal>" }] and stop reason remains stop.
  • Evidence after fix: all six focused refusal tests pass, including packages/ai surfaces aggregated chat-completions message.refusal as visible assistant text.
  • Control check: normal content-only completions chunks still produce text as before; Responses refusal handling is untouched.
  • Observed result after fix: refusal-only completions turns yield visible assistant text matching the provider refusal string for both delta and message forms on both assemblers.
  • What was not tested: live api.openai.com network call that returns a real safety refusal (no OPENAI_API_KEY in 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.refusal completeness fix from review.

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: S labels Jul 9, 2026
@clawsweeper

clawsweeper Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 8, 2026, 10:58 PM ET / 02:58 UTC.

Summary
The PR surfaces OpenAI chat-completions refusal fields as visible assistant text in the agent transport and packages/ai assembler, with delta and aggregated-message regression tests.

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
Relationship: fixed_by_candidate
Canonical: #102321
Summary: The linked issue is the canonical exact report for OpenAI chat-completions refusal fields resolving to empty assistant content; this PR and three sibling PRs are competing candidate fixes for the same root cause.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P2] Resolve or explain the failing checks-node-compact-small-5 CI shard before merge.
  • Pick one canonical PR for the linked issue and close the duplicate candidates after landing.

Risk before merge

  • [P1] A current CI shard, checks-node-compact-small-5, is failing on the PR head and should be understood or rerun before merge.
  • [P1] Several open sibling PRs target the same linked issue, so maintainers should land one canonical branch and close the rest after the fix merges.

Maintainer options:

  1. Decide the mitigation before merge
    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.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • No automated repair is queued; maintainers should choose the canonical duplicate PR and resolve or classify the failing CI shard before merge.

Maintainer decision needed

  • Question: Which open candidate should maintainers treat as the canonical fix for the chat-completions refusal-drop issue once CI is understood?
  • Rationale: The code direction is no longer blocked by this review, but there are multiple live PRs for the same issue and only one should land.
  • Likely owner: steipete — The decision crosses the shared packages/ai runtime and duplicate landing choice, where recent history points to shared AI runtime ownership.
  • Options:
    • Use this PR as canonical (recommended): This PR covers both agent and packages/ai paths, includes delta and message-form tests, and now has contributor runtime proof; merge it after CI is clear and close the duplicate candidates.
    • Choose a sibling PR: A maintainer may prefer a sibling branch if its proof, CI, or final diff is stronger, but that branch should be reviewed as the sole landing candidate.
    • Request consolidation: Ask the contributors to consolidate the best pieces into one branch if maintainers do not want to pick between duplicate implementations directly.

Security
Cleared: The diff only changes local TypeScript stream assembly and tests; it does not alter dependencies, workflows, secrets, package metadata, install scripts, or package resolution.

Review details

Best 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 changes

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The contributor added copied runtime proof from a local HTTP SSE server through the production OpenAI SDK request/parse path for all four refusal shapes; no live OpenAI endpoint was used, but the changed protocol path is demonstrated.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The contributor added copied runtime proof from a local HTTP SSE server through the production OpenAI SDK request/parse path for all four refusal shapes; no live OpenAI endpoint was used, but the changed protocol path is demonstrated.

Label justifications:

  • P2: This is a normal-priority provider output bug fix with limited blast radius: refusal-only OpenAI chat-completions replies can render as blank assistant turns on current main.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The contributor added copied runtime proof from a local HTTP SSE server through the production OpenAI SDK request/parse path for all four refusal shapes; no live OpenAI endpoint was used, but the changed protocol path is demonstrated.
  • proof: sufficient: Contributor real behavior proof is sufficient. The contributor added copied runtime proof from a local HTTP SSE server through the production OpenAI SDK request/parse path for all four refusal shapes; no live OpenAI endpoint was used, but the changed protocol path is demonstrated.
Evidence reviewed

PR surface:

Source +26, Tests +150. Total +176 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 2 35 9 +26
Tests 2 151 1 +150
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 4 186 10 +176

What I checked:

  • Current main agent gap: Current main normalizes choice.delta ?? choice.message in processOpenAICompletionsStream, then handles content, reasoning, and tool calls without reading choiceDelta.refusal, so refusal-only chunks can leave the assistant turn empty. (src/agents/openai-transport-stream.ts:3135, 30b3a7cdeda9)
  • Current main shared-provider gap: Current main streamOpenAICompletions only enters the parser for choice.delta and handles content/tool calls/reasoning without refusal handling, so the packages/ai sibling path has the same output-drop class. (packages/ai/src/providers/openai-completions.ts:427, 30b3a7cdeda9)
  • PR head fixes both assemblers: At PR head, the agent transport appends string refusal text through the visible text partitioner and the packages/ai path normalizes choice.delta ?? choice.message before content, refusal, tool-call, and reasoning handling. (src/agents/openai-transport-stream.ts:3169, 1e49e5005dc3)
  • Regression coverage: PR head adds refusal tests for agent delta/message forms and packages/ai delta/message forms, including the previously raised shared-stream message-refusal gap. (packages/ai/src/providers/openai-completions.test.ts:255, 1e49e5005dc3)
  • Responses parity: The existing Responses path already treats response.refusal.delta and finalized contentPart.refusal as visible text, supporting the narrow parser-parity fix rather than a broader fallback-policy change. (src/agents/openai-transport-stream.ts:1635, 30b3a7cdeda9)
  • Upstream contract: The OpenAI Node SDK v6.45.0 chat completions types define assistant message refusal?: string | null and streamed delta refusal?: string | null, so the parsed field is an upstream response shape. (openai-node/src/resources/chat/completions/completions.ts:610)

Likely related people:

  • vincentkoc: Blame on the affected parser loops resolves to a recent carrier commit, and PR 60264 introduced the OpenAI transport adapter seam that owns this runtime path. (role: recent area contributor and transport-seam introducer; confidence: high; commits: 6f6c3e260222, 4798e125f40c; files: src/agents/openai-transport-stream.ts, packages/ai/src/providers/openai-completions.ts)
  • steipete: PR 99059 extracted the reusable packages/ai provider runtime, and PR 102009 recently touched the shared OpenAI-completions provider path. (role: shared AI runtime package owner and recent adjacent contributor; confidence: high; commits: 062f88e3e3af, 4bf70be01a21; files: packages/ai/src/providers/openai-completions.ts, packages/ai/src/providers/register-builtins.ts)
  • balajisiva: PR 61534 fixed a nearby OpenAI-compatible completions parser issue involving silent output loss around vLLM reasoning and empty tool-call shapes. (role: adjacent bug-fix author; confidence: medium; commits: efab9763dc87; files: src/agents/openai-transport-stream.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (3 earlier review cycles)
  • reviewed 2026-07-09T01:53:45.042Z sha a250cb0 :: needs real behavior proof before merge. :: [P2] Handle message refusals in the shared stream
  • reviewed 2026-07-09T02:33:54.362Z sha db6db43 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-09T02:48:52.570Z sha 1e49e50 :: needs real behavior proof before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. labels Jul 9, 2026
@wuqxuan
wuqxuan force-pushed the grok/openai-completions-refusal-102321 branch from a250cb0 to db6db43 Compare July 9, 2026 02:27
@clawsweeper clawsweeper Bot removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 9, 2026
@wuqxuan

wuqxuan commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Added real-behavior proof beyond the mock chunk tests.

  • Ran the production createOpenAICompletionsTransportStreamFn() and streamOpenAICompletions() paths against a local HTTP SSE server that returned refusal-only OpenAI-compatible chunks.
  • Verified both refusal shapes on both surfaces:
    • agent-delta -> stopReason=stop, visible text I cannot help with that.
    • agent-message -> stopReason=stop, visible text Requests like this are not allowed.
    • ai-delta -> stopReason=stop, visible text I cannot help with that.
    • ai-message -> stopReason=stop, visible text Requests like this are not allowed.
  • Re-ran the focused regression suite:
    • node scripts/run-vitest.mjs src/agents/openai-transport-stream.test.ts packages/ai/src/providers/openai-completions.test.ts -t refusal --maxWorkers=2
    • Result: 3 files passed, 6 tests passed, 637 skipped.

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.

@wuqxuan

wuqxuan commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 9, 2026
@steipete steipete self-assigned this Jul 9, 2026
@vincentkoc vincentkoc self-assigned this Jul 9, 2026
@vincentkoc

Copy link
Copy Markdown
Member

Land-ready maintainer review complete on exact head 1e49e5005dc318773a214a76e4ed4fec42c97548.

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:

  • AWS Crabbox cbx_2851fc865f80, run run_012f426e336b: pnpm test:serial packages/ai/src/providers/openai-completions.test.ts src/agents/openai-transport-stream.test.ts passed, 343/343 tests.
  • Same exact-head AWS Crabbox, run run_e55859b20472: pnpm check:changed passed, including production/test types, changed-file lint, import-cycle, dependency, database-first, and boundary guards.
  • Fresh Codex autoreview against current origin/main: clean, no accepted/actionable findings.
  • GitHub Actions run https://github.com/openclaw/openclaw/actions/runs/28990102135 attempt 2 passed on the exact PR head; the previously failing compact shard passed on rerun.
  • Native scripts/pr prepare-run 102344 completed with hosted_exact_or_recent_rebase; the prepared tree exactly matches the contributor head.

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.

@vincentkoc
vincentkoc merged commit 3fd5251 into openclaw:main Jul 9, 2026
180 of 186 checks passed
@vincentkoc

Copy link
Copy Markdown
Member

Merged via squash.

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 10, 2026
…law#102344)

* fix: surface OpenAI chat-completions refusal as assistant text

* fix: cover message.refusal on packages/ai completions path
steipete pushed a commit that referenced this pull request Jul 11, 2026
* fix: surface OpenAI chat-completions refusal as assistant text

* fix: cover message.refusal on packages/ai completions path

(cherry picked from commit 3fd5251)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

openai-completions: chat-completions refusal field dropped, assistant turn resolves to empty content

3 participants