fix: address CR review issues for v3.0.1#27
Merged
john-zhh merged 6 commits intofeat/v3.0.1from Mar 11, 2026
Merged
Conversation
- Use sync.Once to close shutdownCh so concurrent POST /api/v1/daemon/shutdown requests cannot race to close an already-closed channel (panic) - Copy SessionInfo struct values (not pointers) before releasing the read lock in handleGetSessions to eliminate the data race window between unlock and JSON encoding Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When startDaemonBackground encounters ErrLockContention it waits for the lock holder to finish, then checks IsDaemonRunning. Previously, if the holder's launch failed, two or more waiters would both fall through to the child-process start path without holding the lock, causing concurrent duplicate starts. Now return an explicit error so the caller retries. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
LoadBalancer.Select was called with the same profile key for both scenario-route and default-fallback selections. Each call advanced the shared profile counter, so scenario traffic would silently perturb the default provider order. Pass a distinct key (profile:scenario:<name>) when selecting for a scenario route so each scenario and the default pool maintain independent counters. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…fter retry When a provider returns "input is required" and the proxy retries via /responses, copyResponseFromResponsesAPI only converted to Anthropic format. An openai-chat client would receive raw Responses API JSON/SSE. - Add ResponsesAPIToOpenAIChat (non-streaming) transform - Add transformResponsesAPIToOpenAIChat (streaming) in StreamTransformer - Fix TransformSSEStream short-circuit: openai-responses → openai-chat requires conversion even though both normalize to "openai" - Update copyResponseFromResponsesAPI to branch on client format Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…etry coverage - loadbalancer_test.go: the per-profile isolation test had an empty if body so a counter-pollution regression would never be caught; add t.Errorf - server_test.go: add TestResponsesAPIRetryOpenAIChat to cover the path where an openai-chat client retries via /responses and must receive a Chat Completions response, not a raw Responses API payload Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ractor - ResponsesAPIToOpenAIChat: text, tool_call, incomplete status cases - transformResponsesAPIToOpenAIChat: text stream, tool_call stream - sseUsageExtractor: parses message_start/message_delta, empty sessionID, Close delegation - Fix transformResponsesAPIToOpenAIChat to flush buffered response.completed event when stream ends without trailing blank line (mirrors the existing flush logic in transformResponsesAPIToAnthropic) proxy: 81.8% (was 79.0%), transform: 87.1% (was 79.5%) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addresses all 7 issues raised in the code review for PR #26.
copyResponseFromResponsesAPInow branches on client format and transforms to Chat Completions foropenai-chatclients.StreamTransformershort-circuit fixed to handleopenai-responses → openai-chatcross-format conversion. AddedResponsesAPIToOpenAIChatandtransformResponsesAPIToOpenAIChat.profile:scenario:<name>as the counter key, keeping scenario and default pool counters independent.sseUsageExtractorwraps the SSE response body and parsesmessage_start/message_deltausage events in-flight, callingUpdateSessionUsageon EOF.sync.Oncereplaces the racyselect/closeso concurrentPOST /daemon/shutdowncannot double-closeshutdownCh.handleGetSessionsnow copiesSessionInfostruct values (not pointers) under the read lock before encoding.return fmt.Errorf(...)when daemon didn't start after waiting, preventing two waiters from racing intostartDaemonBackground.TestLoadBalancer_RoundRobinPerProfileIsolationnow has a realt.Errorfassertion. AddedTestResponsesAPIRetryOpenAIChatfor the openai-chat retry path.Test plan
go test -race ./...)TestResponsesAPIRetryOpenAIChatpasses and catches the bug🤖 Generated with Claude Code