fix(runtime): route OpenCode Go Muse Spark through Responses - #3549
Conversation
Astro-Han
left a comment
There was a problem hiding this comment.
Reviewed exact head 2cfa6418263b25f079598592d9dea207f01db2d6 — required test is completed / success bound to that SHA. No P0–P2. One P3 below.
What this fixes, and why the shape is right. The opencode.ai zen/go gateways close muse-spark-1.2-contributor's SSE at [DONE] without a finish_reason, so the AI SDK synthesizes other/unknown and a perfectly normal completion was being reported as truncation / ProviderUnavailable. The fix could easily have been a provider-wide leniency switch; it is not. finishReasonLenientEofModelIds is keyed per model, contains exactly one id, and the comment says why — the defect is a model-specific gateway behavior, not a property of the provider. That distinction is the part worth keeping.
The guard is genuinely narrow — I checked each conjunct against the source rather than taking the shape on faith. The rewrite at model-adapter.ts fires only when all of lenientEofModel && !failure && sawFinish && (raw === 'other' || raw === 'unknown') hold, and it maps to 'tool-calls' when the step produced calls so ai-sdk-backend step-limit accounting keeps working. So:
- a stream that errored is untouched (
!failure), - a stream that ended without a finish event is untouched (
sawFinish) — which is the actual truncation case this check exists to catch, - any other resolved reason passes through unchanged,
opencode-freeis unaffected:muse-spark-1.2-contributor-freelives in the separateOPENCODE_FREE_BROKEN_MODEL_IDSveto list and is never given the lenient set.
Single authority for the model set (provider-registry), runtime only consumes the providerFinishReasonLenientEof helper, and there is no second finish-reason parser anywhere in the change.
[P3] rawFinishReason in token-usage telemetry is no longer raw for these models.
normalizeAiSdkUsage(sdkUsage, { rawFinishReason: finishReason }) is now passed the rewritten reason, and that value propagates through ai-sdk-backend into the emitted tokenUsage.rawFinishReason. Before this change that field always carried what the provider actually resolved to; for a lenient-EOF turn it will now report stop (or tool-calls) even though the gateway sent no finish reason at all.
Nothing breaks — but this is precisely the population you would want to be able to count later, and after this change the telemetry can no longer distinguish "the model said stop" from "we decided it was a stop". Passing rawResolvedFinishReason to normalizeAiSdkUsage while keeping the rewritten value for control flow would preserve both, and costs one identifier.
Non-blocking; fine to fold into a follow-up.
AI-assisted review. Each claim above was verified against the source at this exact head. Under CONTRIBUTING.md §Review this does not count as the required independent human review — merge still needs a human committer other than the author to give LGTM and to decide.
… telemetry The lenient-EOF rewrite (stop/tool-calls) is used for control flow, but normalizeAiSdkUsage now records rawResolvedFinishReason, so telemetry can still distinguish 'the model said stop' from 'we decided it was a stop'. Addresses P3 review comment on apache#3549.
|
@Astro-Han Thanks for the review! Fixed in d8fdd9e — telemetry now keeps the provider-resolved reason, and the lenient test asserts it. |
Astro-Han
left a comment
There was a problem hiding this comment.
Reviewed at exact head d8fdd9e8a. Hosted test is green on this head; a clean Core → Storage → MCP → Runtime build, 37/37 core tests and 207/207 backend tests all pass locally.
NO-GO — one [P1]. The premise the patch rests on does not hold at the wire layer.
[P1] sawFinish is not evidence that the provider sent [DONE]
Two library facts, verified in the installed dependencies at this head:
@ai-sdk/provider-utilsdiscards the sentinel: in its SSE transform,if (data === "[DONE]") { return; }— it is never enqueued, so nothing downstream can observe it.@ai-sdk/openai-compatiblesynthesizes a finish inflush(), which runs on every ordinary response-body EOF, not only after[DONE].
So by the time this code sees a finish, a genuine [DONE] and a bare socket EOF are indistinguishable. A real-wire probe against a local SSE server using this repository's own installed stack collapses three different transports into the same upper-layer shape:
- content followed by
[DONE]; - partial text, clean EOF before
[DONE]; - a complete tool call, clean EOF before
[DONE].
Under this patch, case 2 becomes end_turn — a truncated answer reported as a normal stop — and case 3 additionally executes the truncated generation's tool call.
The patch also omits the "content present" guard its description implies, so an immediate [DONE] with no content becomes a successful empty turn.
Treating a clean EOF as a normal stop needs affirmative evidence that the provider actually terminated the response. That evidence has to be captured where [DONE] still exists — above the layer that erases it — or obtained some other way. Inferring it from a synthesized finish cannot distinguish the three cases above.
Interaction with #3434, which changes that PR's advice too
#3434's execution guard sees the same synthesized finish, and currently lets an ambiguous other borrow the resolved provider outcome. If this PR's rewrite makes that outcome tool-calls, the guard would authorize exactly the truncated tool call it exists to block.
That also retracts a recommendation we published on #3434: we suggested making the post-rewrite ModelStepOutcome the sole terminal-reason authority. That advice was wrong once a rewrite like this one can enter the picture — tool safety must retain unrewritten evidence rather than consuming a settlement another layer may have rewritten. A correction has been posted there.
Verification note
The wire probe uses the repository's installed @ai-sdk/openai-compatible and ai packages against a local SSE server, so it exercises the exact layer where [DONE] is erased and the finish is synthesized, rather than a mock of it.
The gateway serves muse-spark-1.2-contributor correctly through the Responses API, while its Chat Completions stream lacks a reliable terminal finish reason. Pin this provider/model pair to Responses instead of weakening the shared truncation guard, and cover both the scoped protocol choice and final request endpoint. Fixes apache#3548 Generated-by: OpenAI Codex
d8fdd9e to
09de81c
Compare
|
@Astro-Han Thank you for taking the time to trace this through the installed AI SDK dependencies and build a wire-level reproduction. Your review was correct, and I’m sorry that my previous approach addressed the symptom without identifying the actual routing problem. I incorrectly treated the SDK-synthesized After investigating again, I found that the real issue is the API route: Maka was sending I’ve reworked the PR accordingly: the lenient-EOF logic has been removed, the strict truncation and tool-safety checks remain unchanged, and this provider/model pair is now routed to Thank you again for catching this and for the detailed explanation—it led me to the correct layer for the fix. |
jackwener
left a comment
There was a problem hiding this comment.
Reviewed exact head 09de81c3446f0c18ea6914e235b4ee3fce7d4cb6 against base 4852d4922022581740e55f3244c240fbd005100c and current main. GO: no P0–P3 findings.
The earlier lenient-EOF design is gone, so the old fail-open path and its telemetry P3 no longer apply. This head changes only protocol selection for the exact opencode-go + muse-spark-1.2-contributor pair. The generated per-model provider override already selects @ai-sdk/openai; resolveModelRuntime() therefore uses the native OpenAI adapter, the new protocol predicate selects Responses, and getAIModel() correctly sends to https://opencode.ai/zen/go/v1/responses. The same model id under opencode, and other OpenCode Go models, remain on Chat. No stream parsing, EOF inference, retry, or tool-execution gate is relaxed.
Verification: clean Core → Storage → MCP → Runtime build; focused Core/Runtime suites 23/23; diff check clean; clean merge-tree against main@183fe9fb32b794878bf8dfa27e6d9ff5eed7dbb4. Hosted test is completed/success on this exact SHA. GitHub’s APPROVED state is from stale head 2cfa6418, so this COMMENT records the exact-head technical judgment but does not treat that old approval as a current-head gate.
AI-assisted review; the reviewer is accountable for the exact-head control-flow and request-URL verification.
Astro-Han
left a comment
There was a problem hiding this comment.
Gate check at exact head 09de81c3: hosted test is terminal completed/success (the only check this path set triggers), mergeable=true, and there are no unresolved review threads.
The change is a single exact-equality allowlist entry in openAiAdapterApiProtocol (packages/core/src/model-metadata.ts:140) plus its two regression suites. Because the predicate compares id === 'muse-spark-1.2-contributor' against the trimmed model id, opencode and every other opencode-go model keep the Chat protocol — no EOF, retry, or tool-gate leniency is widened.
Approving. The earlier APPROVED review on this PR was bound to 2cfa6418, so this re-approves at the current head.
…3549) The gateway serves muse-spark-1.2-contributor correctly through the Responses API, while its Chat Completions stream lacks a reliable terminal finish reason. Pin this provider/model pair to Responses instead of weakening the shared truncation guard, and cover both the scoped protocol choice and final request endpoint. Fixes apache#3548 Generated-by: OpenAI Codex
Summary
opencode-go/muse-spark-1.2-contributorfailed in Maka because the model resolved to the native OpenAI adapter but defaulted to the Chat Completions wire. That endpoint returns content without a reliable terminalfinish_reason, so Maka's strict stream settlement correctly reports the ambiguous EOF as truncated.Route this exact provider/model pair through the OpenAI Responses API instead. The Responses endpoint provides an explicit terminal response event, so no EOF inference or relaxation of Maka's truncation and tool-safety checks is needed.
This matches the model-specific routing correction merged in can1357/oh-my-pi#8980.
Fixes #3548
Verification
packages/coremodel-metadata.test.js: 10 passedpackages/runtimeresponses-wire-contract.test.js: 13 passedhttps://opencode.ai/zen/go/v1/responsesbiome checkon all changed files: passedReadtool completed successfully and returned the expected package nameNot run: full-repository suite, Desktop, Playwright.
Review focus
The override is scoped to
opencode-go/muse-spark-1.2-contributor, the only affected model currently exposed by Maka's OpenCode Go catalog. Other OpenCode providers and models keep their existing protocol selection.This change does not modify stream parsing, finish-reason settlement, retry behavior, or tool-call authorization. Ambiguous Chat Completions EOFs remain fail-closed.
AI use
Select exactly one:
Tools and scope:
The final commit carries a
Generated-by: OpenAI Codextrailer.Checklist
Does this PR entail a change in behavior?