Skip to content

fix(openai): map Responses refusal chunks during tool calls#101433

Closed
WOLIKIMCHENG wants to merge 2 commits into
openclaw:mainfrom
WOLIKIMCHENG:fix/openai-responses-refusal-chunks
Closed

fix(openai): map Responses refusal chunks during tool calls#101433
WOLIKIMCHENG wants to merge 2 commits into
openclaw:mainfrom
WOLIKIMCHENG:fix/openai-responses-refusal-chunks

Conversation

@WOLIKIMCHENG

@WOLIKIMCHENG WOLIKIMCHENG commented Jul 7, 2026

Copy link
Copy Markdown

Fixes #101059

What Problem This Solves

Fixes an issue where users running OpenAI Responses models with strict tool usage would get a generic tool-call failure instead of a handled refusal when the provider emitted refusal chunks during a tool call stream.

Why This Change Was Made

This change teaches the OpenAI Responses stream parsers to treat refusal chunks that arrive while a function call is in progress as provider refusals instead of finishing an empty or partial tool call. The fix is intentionally narrow: it updates both the shared @openclaw/ai provider path and the agent transport sibling path, while preserving the existing stop-reason mapping for non-refusal completions.

User Impact

Users now get a stable provider-refusal error instead of a cryptic tool-call parsing failure when an OpenAI Responses model refuses a tool call mid-stream. This keeps refusal handling consistent across the shared provider adapter and the agent transport path, and avoids surfacing a broken partial tool invocation as if it were a normal tool call.

Evidence

Focused local validation passed on the touched paths:

OPENCLAW_VITEST_MAX_WORKERS=2 node scripts/run-vitest.mjs run \
  packages/ai/src/providers/openai-responses-shared.test.ts \
  src/agents/openai-transport-stream.test.ts

Results:

  • test/vitest/vitest.unit.config.ts: 35 passed
  • test/vitest/vitest.agents.config.ts: 296 passed
  • [test] passed 2 Vitest shards in 56.22s

Additional checks:

  • git diff --check passed

Regression coverage added for both stream parser paths:

  • shared OpenAI Responses provider path now converts function-call refusal deltas into provider_refusal
  • agent transport path now does the same and drops the incomplete tool-call block before finalization

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

clawsweeper Bot commented Jul 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 7, 2026, 3:47 AM ET / 07:47 UTC.

Summary
The PR adds an OpenAI Responses refusal helper and updates the shared and agent Responses stream parsers/tests to map refusal deltas during function-call streaming to provider-refusal diagnostics.

PR surface: Source +91, Tests +135. Total +226 across 6 files.

Reproducibility: Source-level yes: current main only handles response.refusal.delta for message items while function-call streams are finalized separately. A live OpenAI stream or captured SSE fixture for the exact sequence is still missing.

Review metrics: none identified.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #101059
Summary: This PR is the linked candidate fix for the canonical OpenAI Responses refusal-handling bug, but it still needs a lifecycle fix and real behavior proof before it can resolve the issue.

Members:

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

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦪 silver shellfish
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

  • [P1] Preserve the refusal error message through the shared and agent Responses lifecycle wrappers and add lifecycle-level regression coverage.
  • [P1] Add redacted real behavior proof from a live Responses run, captured SSE replay, terminal output, logs, or a reproducible validation transcript.

Proof guidance:

  • [P1] Needs real behavior proof before merge: Only focused parser/unit-test output is provided; add redacted live output, terminal/log proof, or a captured Responses SSE replay showing the after-fix refusal path, then update the PR body for re-review or ask a maintainer to comment @clawsweeper re-review.

Risk before merge

  • [P1] Public shared and agent Responses wrappers still overwrite parser-level provider-refusal state with a generic unknown-error message, so the user-visible fix is not delivered yet.
  • [P1] The PR body provides parser/unit-test output only; no redacted live Responses stream, captured SSE replay, or public lifecycle transcript proves the after-fix refusal path.

Maintainer options:

  1. Decide the mitigation before merge
    Preserve provider-refusal diagnostics through the shared and agent Responses lifecycle wrappers, cover those entrypoints with regression tests, then add redacted real Responses or captured-SSE proof before merge.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] Manual review and contributor follow-up are needed because the branch still has a lifecycle correctness finding and the external contributor must provide real behavior proof.

Security
Cleared: The diff changes OpenAI Responses stream parsing and tests only; I found no dependency, workflow, secret-handling, permission, or supply-chain surface change.

Review findings

  • [P2] Preserve refusal errors through the streaming lifecycle — packages/ai/src/providers/openai-responses-shared.ts:992
Review details

Best possible solution:

Preserve provider-refusal diagnostics through the shared and agent Responses lifecycle wrappers, cover those entrypoints with regression tests, then add redacted real Responses or captured-SSE proof before merge.

Do we have a high-confidence way to reproduce the issue?

Source-level yes: current main only handles response.refusal.delta for message items while function-call streams are finalized separately. A live OpenAI stream or captured SSE fixture for the exact sequence is still missing.

Is this the best way to solve the issue?

No: accumulating refusal deltas in the parser is a plausible layer, but the public stream lifecycle still turns parser-level errors into a generic thrown error. The narrower complete fix is to preserve the existing refusal message through the wrappers and test those entrypoints.

Full review comments:

  • [P2] Preserve refusal errors through the streaming lifecycle — packages/ai/src/providers/openai-responses-shared.ts:992
    This sets output.errorMessage to the provider-refusal text, but the real shared and agent stream entrypoints still check output.stopReason === "error", throw new Error("An unknown error occurred"), and then overwrite the output error message in their catch paths. The new tests call the parser directly, so users can still receive the generic error instead of the handled refusal; preserve the parser error through those wrappers and add lifecycle-level coverage. This was raised in the previous review cycle and remains at the current head.
    Confidence: 0.92

Overall correctness: patch is incorrect
Overall confidence: 0.89

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 5c4b63964cc3.

Label changes

Label justifications:

  • P2: This PR targets a normal-priority OpenAI Responses provider bug with limited blast radius, and the remaining blocker is focused runtime correctness plus proof.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦪 silver shellfish.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Only focused parser/unit-test output is provided; add redacted live output, terminal/log proof, or a captured Responses SSE replay showing the after-fix refusal path, then update the PR body for re-review or ask a maintainer to comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +91, Tests +135. Total +226 across 6 files.

View PR surface stats
Area Files Added Removed Net
Source 4 101 10 +91
Tests 2 135 0 +135
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 6 236 10 +226

What I checked:

Likely related people:

  • steipete: Authored the merged @openclaw/ai extraction that owns the shared provider runtime boundary containing the Responses shared parser. (role: recent package extraction owner; confidence: medium; commits: 062f88e3e3af; files: packages/ai/src/providers/openai-responses-shared.ts, packages/ai/src/providers/openai-responses.ts)
  • amersheeny: Authored the merged OpenAI Responses snapshot-collapse and stream event lifecycle fixes across the same shared and agent parser surfaces. (role: adjacent OpenAI Responses stream fixer; confidence: medium; commits: 9fbc8a74efa7; files: packages/ai/src/providers/openai-responses-shared.ts, src/agents/openai-transport-stream.ts)
  • osolmaz: Authored recent first-event timeout and stream lifecycle work in the agent transport path that is relevant to preserving parser-level errors through wrapper finalization. (role: adjacent agent stream lifecycle contributor; confidence: medium; commits: 530d576cc642; 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 (1 earlier review cycle)
  • reviewed 2026-07-07T07:06:58.563Z sha e78290e :: needs real behavior proof before merge. :: [P2] Preserve refusal errors through the streaming lifecycle

@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 7, 2026
@yetval yetval mentioned this pull request Jul 9, 2026
Closed
This was referenced Jul 9, 2026
Closed
Closed
Merged
Closed
@vincentkoc vincentkoc self-assigned this Jul 11, 2026
@vincentkoc

vincentkoc commented Jul 11, 2026

Copy link
Copy Markdown
Member

Closing because the patch is based on an unproven and contract-invalid wire shape. Official refusal delta events identify message content with item, output, and content indexes; this branch omits that identity and attaches refusal to whichever function call is current. That can corrupt parallel tool calls, and the lifecycle wrapper still replaces the refusal with a generic error. Please reopen with a redacted real SSE capture and a current-main implementation routed by event identity.

@vincentkoc vincentkoc closed this Jul 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@vincentkoc vincentkoc

Labels

agents Agent runtime and tooling P2 Normal backlog priority with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: M status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: OpenAI streaming provider fails to map refusal chunks to ModelRefusalError

2 participants

@WOLIKIMCHENG @vincentkoc