fix(providers/openai): send explicit stream:false in chat completion body#526
Conversation
…body
OpenAI API spec defines `stream` as defaulting to false when absent, so
the current code (which omits it) should yield JSON. Some OpenAI-compatible
proxies disagree and default to text/event-stream, which crashes the
`response.json()` parser below with:
Unexpected token 'd', "data: {"id"... is not valid JSON
After a few of these in a row, the resilient wrapper's circuit breaker
trips and all subsequent compression calls fail with `circuit_breaker_open`,
silently disabling LLM-backed compression / summarisation / reflection.
Reproduced upstream in decolua/9router#1260: 9Router's `handleChatCore`
returns SSE unless `stream: false` is explicit. PR
decolua/9router#1272 fixes the proxy side, but
sending the field explicitly here is defensive — other OpenAI-compatible
endpoints (older self-hosted proxies, vLLM compat shims, …) hit the same
spec gap.
No behavior change for spec-compliant endpoints (openai.com, Azure
OpenAI, well-behaved proxies): they already default to non-streaming
when `stream` is absent, so making it explicit is a no-op there.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
@Ptah-CT is attempting to deploy a commit to the rohitg00's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe ChangesOpenAI Provider Streaming Fix
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Thanks @Ptah-CT — circuit-breaker silently disabling LLM compression is a nasty failure mode; appreciate the load-bearing comment so future readers see the why. Merged. |
…uard (#588) Three concerns surfaced when auditing PRs merged since v0.9.21: 1) Graph parser regex from #494 was correct for self-closing tags ONLY when they're the only entity in the document. With a self-closing entity followed by an explicit-close entity, the greedy `[^>]*` ate the trailing `/` of the self-close, the alternation fell through to the explicit-close branch, and `[\s\S]*?` ran ahead to the *next* `</entity>` — merging two entity declarations into one match and silently dropping a node. Switch to lazy `[^>]*?` so the self-closing alternation gets first chance. Test from #494 (which was failing on main but I didn't notice) now passes. 2) #386 shipped `${AGENTMEMORY_URL}` / `${AGENTMEMORY_SECRET}` placeholders in plugin/.mcp.json. Claude Code and Cursor expand those at MCP launch time; some hosts pass the literal string through. A truthy literal `"${AGENTMEMORY_URL}"` defeats the existing `|| DEFAULT_URL` fallback and would have the shim POST to `${AGENTMEMORY_URL}/agentmemory/...` (DNS failure). Add a defensive guard in src/mcp/rest-proxy.ts that strips any value of the form `${...}` so the fallback engages. Mirror in src/mcp/standalone.ts's mode-announce log line. 3) CHANGELOG entries for all PRs landed since v0.9.21 (#321, #325, #386, #454, #494, #526, #542, #560, #561, #562, #564, #567) plus the regex + env-guard hardening here. Validation: - npm test → 98 files, 1091 tests pass - npm pack → 142 files, fresh install clean, iii-sdk@0.11.2 pinned, plugin/ shipped with hooks/scripts/skills/opencode/ - New test file covers 8 placeholder cases (unset, empty, `${VAR}` literal, mismatched braces, real values with $, unclosed `${`, no-braces `$VAR`).
Summary
OpenAIProvider.callomits thestreamfield, relying on the OpenAI spec default offalse. Some OpenAI-compatible proxies disagree and returntext/event-streaminstead of JSON, which crashesresponse.json()with:After a few of these in a row,
ResilientProvider's circuit breaker trips and every subsequent compression / summarisation / reflection fails withcircuit_breaker_open— silently disabling all LLM-backed memory enrichment.This PR sends
stream: falseexplicitly in the request body.Why this is upstream-worthy even with a proxy-side fix
Reproduced concretely with 9Router, an OpenAI-compatible proxy that ships with default
streamresolved asbody.stream !== false(i.e.undefined !== false→true). I opened decolua/9router#1272 against that, but defensivestream: falsehere protects against:For spec-compliant endpoints (openai.com, Azure OpenAI, well-behaved proxies): no behavior change — they already default to non-streaming when
streamis absent, so setting it is a no-op.Repro
Then:
Wrapper log without this PR:
With this PR: response is JSON,
data.choices[0].message.contentextracted, compress succeeds.Test plan
"stream":false, response shape unchangedRelated
decolua/9router#1260— upstream proxy bugdecolua/9router#1272— proxy-side fix PR (defensive, this PR is the agentmemory-side defense in depth)#149/#181— earlier circuit-breaker / Stop-hook recursion fixes; this is the sameResilientProviderpathSummary by CodeRabbit