chore: sync fantasy fork with upstream v0.40.0 and openai-go with v3.50.0 - #27981
Conversation
…50.0 Bumps the charm.land/fantasy replace to coder/fantasy coder_2_33 synced with upstream charmbracelet/fantasy v0.40.0 (coder/fantasy#51), and the openai-go/v3 replace to coder/openai-go coder/pinned rebased onto upstream v3.50.0 (coder/openai-go#10). Fork hacks now superseded upstream and dropped from our diff: truncated stream fail-closed detection (Anthropic + OpenAI Responses), Anthropic EffortXHigh, computer use, thinking effort/display, replay fidelity for signed reasoning and web_search errors, PDF/text documents with sanitized filename titles, refusal finish-reason mapping, gpt-5.5/5.6 Responses routing, the Go 1.25 downgrade, and the openai-go SSE decoder and appendCompact patches. Fork hacks still needed and preserved: OpenAI computer use, Responses replay continuity validation, Anthropic pre-4.6 budget-thinking conversion and effort none, RefusalMetadata parsing, Bedrock cross-region profile mirroring, and openai-go deferred body serialization with the WithJSONSet fix. New upstream features picked up along the way include stream transport retry with in-band SSE error classification, Bedrock auth error refresh, per-message cache markers for OpenAI-compatible models, tool panic recovery, and extra usage fields in provider metadata. Adaptations in this repo: - responses.ResponseOutputItemUnion.Arguments became a union in openai-go v3.50; read the JSON string via .OfString. - Register the new fantasy Call.Headers, ObjectCall.Headers, and ToolResultPart.ClientMetadata fields in the chatdebug coverage map. - Make the integration-test RST listener drain the request before resetting so the client error is deterministic; the new SDK's write path exposed the previous race as sporadic "use of closed network connection" failures.
Remote UAT: PASSDogfooded this PR on a fresh dev.coder.com workspace at commit Validated:
No issues found in the PR's risk surface. Full evidence (screenshots, recording, manifest) attached to the UAT chat: https://dev.coder.com/agents/32b33ee2-68f1-41ec-bb18-3142234e363b Not covered at runtime: Bedrock/Azure/Google providers, AI Bridge intercept beyond unit-test coverage, load/concurrency.
|
Remote UAT round 2 (OpenAI focus): PASSSame chat, same commit Validated with real
Findings (neither blocks this PR):
Evidence attached to the UAT chat: https://dev.coder.com/agents/32b33ee2-68f1-41ec-bb18-3142234e363b
|
UAT round 3: baseline check on main (no regression)Round 2 flagged two OpenAI observations on this branch. To rule out regressions, the same remote UAT agent reran those scenarios on latest main ( Result: both behaviors reproduce identically on main. Neither is introduced by this PR.
The closed-pipe warning is a pre-existing cosmetic issue (candidate for a separate downgrade-to-debug cleanup), and the empty-reasoning default is by-design OpenAI behavior. No fixes needed in this PR. Evidence: UAT chat rounds 1-3 at https://dev.coder.com/agents/32b33ee2-68f1-41ec-bb18-3142234e363b (EVIDENCE-R3.md has the verbatim log lines from main).
|
coder/fantasy#51 merged into coder_2_33 (bb10946892ef) and coder/openai-go#10 merged into coder/pinned (92b5addb22d2). Both merge commits carry trees identical to the previously pinned PR heads, so this is a metadata-only repin onto branch history.
| // Read at least 1 byte so the client has started writing | ||
| // before we RST, ensuring a consistent "connection reset by peer". | ||
| buf := make([]byte, 1) | ||
| _, _ = c.Read(buf) | ||
| // Drain the request before the RST so the client observes a | ||
| // read-side reset rather than a racy body-write failure. | ||
| if req, err := http.ReadRequest(bufio.NewReader(c)); err == nil { | ||
| _, _ = io.Copy(io.Discard, req.Body) | ||
| _ = req.Body.Close() | ||
| } |
There was a problem hiding this comment.
It might be a consequence of the openai ref change. I'll double check
There was a problem hiding this comment.
Fair question, so I measured it. This change is coupled to the openai-go v3.50 bump, not an independent flake fix:
- New SDK + old helper (read 1 byte then RST):
streaming_connection_refusedfails 3+/40 runs withreadfrom tcp ... write: broken pipeoruse of closed network connection, neither of which matches the assertedconnection reset by peer|forcibly closed. - Old SDK + old helper (current main): 160/160 runs pass, so there is nothing to fix on main and a standalone PR would have no defect to point at.
- New SDK + new helper (drain the request, then RST): 160/160 runs pass.
Mechanism: with v3.50 the request body reaches net/http as a reader (the readfrom in the error is the transport streaming it), so an RST fired after 1 byte races the client's body write and surfaces as a write-side error instead of the read-side reset the test asserts. Draining the full request first makes the client deterministically blocked on the response read when the RST lands.
Without this hunk the PR's own CI is flaky, so keeping it here seemed right, but happy to split it if you prefer.
Mux acted on Mike's behalf for this reply.
There was a problem hiding this comment.
That's fine, I was mainly curious about the rationale.
2 participants
Our fantasy fork had drifted far behind upstream charmbracelet/fantasy (base v0.31.0 vs current v0.40.0). This PR updates the pinned forks after reconciling which fork hacks upstream has fixed and which we still need, and adapts this repo to the new APIs.
Fork updates
charm.land/fantasy-> coder/fantasy#51 (merged):coder_2_33synced with upstream v0.40.0, pinned at the merge commitbb10946892ef.github.com/openai/openai-go/v3-> coder/openai-go#10 (merged):coder/pinnedrebased from v3.16.0 onto upstream v3.50.0 (required by upstream fantasy), pinned at the merge commit92b5addb22d2.coder/anthropic-sdk-gopin unchanged; the fantasy fork now tracks the same revision this repo ships.Hack reconciliation summary
Dropped from our fantasy diff (upstream now has equivalents, often stricter): truncated-stream fail-closed detection, Anthropic EffortXHigh / computer use / thinking effort / thinking display, replay fidelity for signed reasoning and web_search errors, PDF and text documents with sanitized filename titles, refusal finish-reason mapping (upstream also maps Bedrock
content_filtered/guardrail_intervened), gpt-5.5/5.6 Responses routing, the Go 1.25 downgrade, and the openai-go SSE decoder and appendCompact patches.Still fork-only and preserved: OpenAI computer use, OpenAI Responses replay continuity validation, Anthropic pre-4.6 budget-thinking conversion plus explicit thinking disable for effort none, Anthropic RefusalMetadata parsing, Bedrock cross-region inference profile region mirroring, and openai-go deferred body serialization with the WithJSONSet fix.
Picked up new upstream features: stream transport retry with in-band SSE error classification, Bedrock expired-credential refresh, per-message cache markers for OpenAI-compatible models, tool panic recovery, extra usage fields in provider metadata, and ClientMetadata on tool results.
Changes in this repo
aibridge/intercept/responses:ResponseOutputItemUnion.Argumentsbecame a union type in openai-go v3.50; read function-call arguments via.OfString(plus test literal updates).coderd/x/chatd/chatdebug: register the new fantasyCall.Headers,ObjectCall.Headers, andToolResultPart.ClientMetadatafields in the normalization coverage map (all skipped).aibridge/internal/integrationtest: make the RST test listener drain the request before resetting the connection. The new SDK's write path exposed the previous 1-byte-read race as sporadicuse of closed network connectionfailures; the fix holds over 40 consecutive runs.go.mod: rewrite the fork provenance comments to describe the post-sync state.Validation
go build ./...andgo vet ./...clean (vet findings identical to base).-count=1) runs of./coderd/x/chatd/...,./aibridge/...,./coderd/aibridged/...,./coderd/database/db2sdk/: 37 packages pass.TestClientAndConnectionErrorstress-tested 40x clean.