From cdc80d6216d2cd074759843531b9229c925fd247 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=94=A1=E5=8F=8A?= <522caiji@gmail.com> Date: Sat, 12 Sep 2026 23:59:17 +0800 Subject: [PATCH] fix: handle CORS preflight and interrupted WorkBuddy tools --- CHANGELOG.md | 4 + internal/api/auth_test.go | 48 +++++ internal/api/server.go | 29 +++ internal/providers/workbuddy/client.go | 11 +- internal/providers/workbuddy/client_test.go | 189 +++++++++++++++++++- internal/providers/workbuddy/credential.go | 2 + internal/providers/workbuddy/payload.go | 147 ++++++++++++++- 7 files changed, 410 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4914f91..3a7e8c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,11 +7,15 @@ Write each change in both `### English` and `### 中文` under `## Unreleased`. ### English +- Allow CORS preflight requests on the OpenAI-compatible endpoints without weakening API-key authentication on actual requests +- Repair WorkBuddy tool history after a canceled tool round so the next turn is not rejected as a broken tool sequence - Send WorkBuddy Deepseek V4.1 Flash thinking as official top-level reasoning fields so streamed thinking comes back - Show WorkBuddy catalog context budgets (default and optional window) on the model list without inventing a Trae Max switch ### 中文 +- OpenAI 兼容接口允许跨域预检请求,但实际请求仍必须通过 API Key 认证 +- WorkBuddy 在工具调用中途停止后,下一轮会修好不完整的 tool 记录,避免被当成工具序列损坏拒绝 - WorkBuddy 的 Deepseek V4.1 Flash 改为发送官方顶层思考字段,流式思考内容可以返回 - 模型列表展示 WorkBuddy 目录里的默认和可选上下文窗口,不套用 Trae 的更大上下文开关 diff --git a/internal/api/auth_test.go b/internal/api/auth_test.go index 0af77b6..aff41db 100644 --- a/internal/api/auth_test.go +++ b/internal/api/auth_test.go @@ -66,6 +66,54 @@ func TestManagementRoutesRequireAPIKey(t *testing.T) { } } +func TestOpenAIEndpointsAllowCORSPreflightWithoutAPIKey(t *testing.T) { + srv := New(config.Config{ + Host: "127.0.0.1", Port: 3010, ProxyAPIKey: "secret", QoderHome: t.TempDir(), DataDir: t.TempDir(), + }) + defer srv.Close() + + for _, path := range []string{ + "/v1/models", + "/v1/chat/completions", + "/v1/messages", + "/v1/responses", + } { + req := httptest.NewRequest(http.MethodOptions, path, nil) + req.Header.Set("Origin", "chrome-extension://example") + req.Header.Set("Access-Control-Request-Method", http.MethodPost) + req.Header.Set("Access-Control-Request-Headers", "authorization, content-type") + rec := httptest.NewRecorder() + srv.Handler().ServeHTTP(rec, req) + if rec.Code != http.StatusNoContent { + t.Fatalf("OPTIONS %s: got %d body=%s", path, rec.Code, rec.Body.String()) + } + if rec.Header().Get("Access-Control-Allow-Origin") != "*" || + rec.Header().Get("Access-Control-Allow-Methods") == "" || + rec.Header().Get("Access-Control-Allow-Headers") == "" { + t.Fatalf("OPTIONS %s missing CORS headers: %v", path, rec.Header()) + } + } + + chat := httptest.NewRequest(http.MethodPost, "/v1/chat/completions", strings.NewReader(`{"model":"m","messages":[{"role":"user","content":"hi"}]}`)) + chat.Header.Set("Origin", "chrome-extension://example") + chatRec := httptest.NewRecorder() + srv.Handler().ServeHTTP(chatRec, chat) + if chatRec.Code != http.StatusUnauthorized { + t.Fatalf("unauthenticated chat: got %d want 401", chatRec.Code) + } + if chatRec.Header().Get("Access-Control-Allow-Origin") != "*" { + t.Fatalf("unauthenticated chat missing CORS headers: %v", chatRec.Header()) + } + + management := httptest.NewRequest(http.MethodOptions, "/api/chat", nil) + management.Header.Set("Origin", "chrome-extension://example") + managementRec := httptest.NewRecorder() + srv.Handler().ServeHTTP(managementRec, management) + if managementRec.Code != http.StatusUnauthorized { + t.Fatalf("management OPTIONS: got %d want 401", managementRec.Code) + } +} + func TestOverviewSummaryReturnsLightweightSnapshot(t *testing.T) { dir := t.TempDir() srv := New(config.Config{ diff --git a/internal/api/server.go b/internal/api/server.go index faf4f46..defd87c 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -172,6 +172,13 @@ func generateAPIKey() (string, error) { func (s *Server) Handler() http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if isOpenAIEndpoint(r.URL.Path) { + setOpenAICORSHeaders(w, r) + if r.Method == http.MethodOptions { + w.WriteHeader(http.StatusNoContent) + return + } + } if s.maintenance.Load() && blocksDuringUpdate(r.URL.Path) { writeErr(w, http.StatusServiceUnavailable, "service_updating", "Service update in progress") return @@ -180,6 +187,28 @@ func (s *Server) Handler() http.Handler { }) } +func isOpenAIEndpoint(path string) bool { + switch path { + case endpoint.ModelsPath, endpoint.ChatCompletionsPath, endpoint.MessagesPath, endpoint.ResponsesPath: + return true + default: + return false + } +} + +func setOpenAICORSHeaders(w http.ResponseWriter, r *http.Request) { + header := w.Header() + header.Set("Access-Control-Allow-Origin", "*") + header.Set("Access-Control-Allow-Methods", "GET, POST, OPTIONS") + requestedHeaders := strings.TrimSpace(r.Header.Get("Access-Control-Request-Headers")) + if requestedHeaders == "" { + requestedHeaders = "Authorization, Content-Type, X-API-Key, X-Requested-With" + } + header.Set("Access-Control-Allow-Headers", requestedHeaders) + header.Set("Access-Control-Expose-Headers", "X-Request-Id, X-Qoder-Account, X-CLI2API-Account, X-CLI2API-Provider") + header.Set("Access-Control-Max-Age", "600") +} + func (s *Server) Close() error { if s.stopLogs != nil { close(s.stopLogs) diff --git a/internal/providers/workbuddy/client.go b/internal/providers/workbuddy/client.go index 7f8a9fc..dad4b56 100644 --- a/internal/providers/workbuddy/client.go +++ b/internal/providers/workbuddy/client.go @@ -570,7 +570,7 @@ func Classify(status int, body string) providers.ClassifiedError { return providers.ClassifiedError{Kind: accounts.KindRateLimit, Status: 429, Message: strings.TrimSpace(body)} case status == 404: return providers.ClassifiedError{Kind: accounts.KindUnavailable, Status: 404, Message: strings.TrimSpace(body)} - case status == 400 || accounts.IsPromptLimitText(body) || accounts.IsInvalidRequestText(body) || isMissingSystemPrompt(body): + case status == 400 || accounts.IsPromptLimitText(body) || accounts.IsInvalidRequestText(body) || isMissingSystemPrompt(body) || isBrokenToolSequence(body): // Request-level rejection (content screening, malformed fields, // missing leading system message): retrying on another account // cannot help and the account is healthy. @@ -583,7 +583,7 @@ func Classify(status int, body string) providers.ClassifiedError { if env.Code == sessionDeadCode || strings.Contains(strings.ToLower(env.Msg), sessionDeadText) { return providers.ClassifiedError{Kind: accounts.KindAuth, Status: 401, Message: "session dead; re-login required"} } - if accounts.IsPromptLimitText(env.Msg) || accounts.IsInvalidRequestText(env.Msg) || isMissingSystemPrompt(env.Msg) || env.Code == missingSystemPromptCode { + if accounts.IsPromptLimitText(env.Msg) || accounts.IsInvalidRequestText(env.Msg) || isMissingSystemPrompt(env.Msg) || isBrokenToolSequence(env.Msg) || env.Code == missingSystemPromptCode || env.Code == toolCallSequenceCode { return providers.ClassifiedError{Kind: accounts.KindInvalidRequest, Status: firstNonEmptyStatus(status, 400), Message: env.Msg} } return providers.ClassifiedError{Kind: accounts.KindUnavailable, Status: 502, Message: env.Msg} @@ -604,6 +604,13 @@ func isMissingSystemPrompt(text string) bool { strings.Contains(lower, fmt.Sprintf("%d", missingSystemPromptCode)) } +func isBrokenToolSequence(text string) bool { + lower := strings.ToLower(text) + return strings.Contains(lower, toolCallSequenceText) || + strings.Contains(lower, "tool_call_sequence_broken") || + strings.Contains(lower, fmt.Sprintf("%d", toolCallSequenceCode)) +} + func catalogErrorBody(body []byte) string { text := strings.TrimSpace(string(body)) if text == "" { diff --git a/internal/providers/workbuddy/client_test.go b/internal/providers/workbuddy/client_test.go index da0cd70..c14ea6d 100644 --- a/internal/providers/workbuddy/client_test.go +++ b/internal/providers/workbuddy/client_test.go @@ -193,6 +193,106 @@ func TestChatNonStreamAggregatesToolsAndReasoning(t *testing.T) { } } +func TestChatNonStreamSendsSingleAndMultiTurnHistories(t *testing.T) { + tests := []struct { + name string + messages []translate.ChatMessage + wantRoles string + wantToolCalls int + wantToolID string + }{ + { + name: "single turn", + messages: []translate.ChatMessage{{Role: "user", Content: "hello"}}, + wantRoles: "system,user", + }, + { + name: "complete multi turn with tools", + messages: []translate.ChatMessage{ + {Role: "user", Content: "look this up"}, + {Role: "assistant", Content: "", ToolCalls: json.RawMessage(`[{"id":"call_1","type":"function","function":{"name":"search","arguments":"{}"}},{"id":"call_2","type":"function","function":{"name":"search","arguments":"{}"}}]`)}, + {Role: "tool", ToolCallID: "call_1", Content: "first result"}, + {Role: "tool", ToolCallID: "call_2", Content: "second result"}, + {Role: "user", Content: "continue"}, + }, + wantRoles: "system,user,assistant,tool,tool,user", + wantToolCalls: 2, + wantToolID: "call_2", + }, + { + name: "interrupted multi turn", + messages: []translate.ChatMessage{ + {Role: "user", Content: "look this up"}, + {Role: "assistant", Content: "", ToolCalls: json.RawMessage(`[{"id":"call_interrupted","type":"function","function":{"name":"search","arguments":"{}"}}]`)}, + {Role: "user", Content: "stop and answer me"}, + }, + wantRoles: "system,user,user", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + payload, _ := Credential{AccessToken: "at", UID: "u1", Domain: "codebuddy.cn", ExpiresAt: 4102444800}.Encode() + store := &memStore{items: map[string][]byte{"acc1": payload}} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != pathChat { + t.Fatalf("path=%s", r.URL.Path) + } + var body map[string]any + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + t.Fatalf("decode request: %v", err) + } + if body["stream"] != true { + t.Fatalf("stream=%v", body["stream"]) + } + if got := strings.Join(messageRoles(body), ","); got != test.wantRoles { + t.Fatalf("roles=%s want=%s messages=%v", got, test.wantRoles, body["messages"]) + } + messages, _ := body["messages"].([]any) + for _, raw := range messages { + message, _ := raw.(map[string]any) + if message["role"] != "assistant" { + continue + } + calls, _ := message["tool_calls"].([]any) + if len(calls) != test.wantToolCalls { + t.Fatalf("tool calls=%v want=%d", message["tool_calls"], test.wantToolCalls) + } + } + if test.wantToolID != "" { + found := false + for _, raw := range messages { + message, _ := raw.(map[string]any) + if message["role"] == "tool" && message["tool_call_id"] == test.wantToolID { + found = true + } + } + if !found { + t.Fatalf("tool result %q not found: %v", test.wantToolID, messages) + } + } + w.Header().Set("Content-Type", "text/event-stream") + _, _ = io.WriteString(w, `data: {"choices":[{"delta":{"content":"ok"},"finish_reason":"stop"}]} + +data: [DONE] + +`) + })) + defer server.Close() + client := NewClient(store) + client.rememberCatalog([]providers.ModelInfo{{NativeModel: "glm-5.2", Capabilities: providers.ModelCapabilities{ReasoningOptions: []string{"none", "medium"}}}}) + client.http = server.Client() + client.http.Transport = rewriteTransport{server: server.URL, round: server.Client().Transport} + + if _, err := client.ChatNonStream(context.Background(), "acc1", translate.ChatRequest{ + Model: "glm-5.2", Messages: test.messages, + }); err != nil { + t.Fatal(err) + } + }) + } +} + func TestChatNonStreamReadLifecycle(t *testing.T) { for _, test := range []struct { name string @@ -211,13 +311,13 @@ func TestChatNonStreamReadLifecycle(t *testing.T) { w.Header().Set("Content-Length", "100000") } w.Header().Set("Content-Type", "text/event-stream") - _, _ = io.WriteString(w, "data: {}\n\n") - w.(http.Flusher).Flush() - if test.cancel { - cancel() - <-r.Context().Done() - return - } + _, _ = io.WriteString(w, "data: {}\n\n") + w.(http.Flusher).Flush() + if test.cancel { + cancel() + <-r.Context().Done() + return + } if test.truncate { return } @@ -602,6 +702,7 @@ func TestErrorMapping(t *testing.T) { {500, "boom", "unavailable"}, {400, `{"code":11101,"msg":"bad request"}`, "invalid_request"}, {200, `{"code":11128,"msg":"first message is not system prompt"}`, "invalid_request"}, + {200, `{"code":11148,"msg":"tool calls and tool results do not match, please start a new conversation and retry","extError":{"code":"tool_call_sequence_broken"}}`, "invalid_request"}, {200, `{"code":12001,"msg":"内容包含敏感信息"}`, "invalid_request"}, {200, `{"code":12002,"msg":"sensitive content detected"}`, "invalid_request"}, {429, `{"code": "insufficient_quota", "msg": "token-limit"}`, "invalid_request"}, @@ -747,9 +848,79 @@ func TestPrepareBodyNormalizesEmptyMessageContent(t *testing.T) { t.Fatal(err) } toolMessages, _ := toolBody["messages"].([]any) - if len(toolMessages) != 2 { - t.Fatalf("tool message was dropped: %v", toolMessages) + if len(toolMessages) != 1 { + t.Fatalf("unmatched tool_calls should be dropped: %v", toolMessages) + } + first, _ := toolMessages[0].(map[string]any) + if first["role"] != "system" { + t.Fatalf("messages=%v", toolMessages) + } +} + +func TestPrepareBodyRepairsInterruptedToolSequence(t *testing.T) { + out := PrepareBody([]byte(`{"model":"m","messages":[ + {"role":"user","content":"look this up"}, + {"role":"assistant","content":"","tool_calls":[{"id":"call_abc","type":"function","function":{"name":"search","arguments":"{\"q\":\"par"}},{"id":"","function":{"name":"search","arguments":""}}]}, + {"role":"user","content":"continue"} + ]}`)) + var body map[string]any + if err := json.Unmarshal(out, &body); err != nil { + t.Fatal(err) + } + roles := messageRoles(body) + if strings.Join(roles, ",") != "system,user,user" { + t.Fatalf("interrupted tool_calls should be dropped: %v", body["messages"]) + } + + paired := PrepareBody([]byte(`{"model":"m","messages":[ + {"role":"user","content":"look this up"}, + {"role":"assistant","content":"","tool_calls":[{"id":"call_abc","type":"function","function":{"name":"search","arguments":"{}"}},{"id":"call_def","type":"function","function":{"name":"search","arguments":"{}"}}]}, + {"role":"tool","tool_call_id":"call_abc","content":""}, + {"role":"user","content":"go on"} + ]}`)) + if err := json.Unmarshal(paired, &body); err != nil { + t.Fatal(err) + } + roles = messageRoles(body) + if strings.Join(roles, ",") != "system,user,user" { + t.Fatalf("partial tool round should be dropped as a unit: %v", body["messages"]) + } + + complete := PrepareBody([]byte(`{"model":"m","messages":[ + {"role":"user","content":"look this up"}, + {"role":"assistant","content":"","tool_calls":[{"id":"call_abc","type":"function","function":{"name":"search","arguments":"{}"}}]}, + {"role":"tool","tool_call_id":"call_abc","content":"ok"}, + {"role":"user","content":"thanks"} + ]}`)) + if err := json.Unmarshal(complete, &body); err != nil { + t.Fatal(err) + } + if strings.Join(messageRoles(body), ",") != "system,user,assistant,tool,user" { + t.Fatalf("complete tool round-trip rewritten: %v", body["messages"]) + } + + orphan := PrepareBody([]byte(`{"model":"m","messages":[ + {"role":"user","content":"hi"}, + {"role":"tool","tool_call_id":"call_orphan","content":"leftover"}, + {"role":"user","content":"again"} + ]}`)) + if err := json.Unmarshal(orphan, &body); err != nil { + t.Fatal(err) + } + if strings.Join(messageRoles(body), ",") != "system,user,user" { + t.Fatalf("orphan tool result kept: %v", body["messages"]) + } +} + +func messageRoles(body map[string]any) []string { + messages, _ := body["messages"].([]any) + roles := make([]string, 0, len(messages)) + for _, item := range messages { + message, _ := item.(map[string]any) + role, _ := message["role"].(string) + roles = append(roles, role) } + return roles } func TestPrepareBodyDropsNullAndEmptyTools(t *testing.T) { diff --git a/internal/providers/workbuddy/credential.go b/internal/providers/workbuddy/credential.go index 25e9373..4c483df 100644 --- a/internal/providers/workbuddy/credential.go +++ b/internal/providers/workbuddy/credential.go @@ -47,6 +47,8 @@ const ( sessionDeadText = "Offline user session not found" missingSystemPromptCode = 11128 missingSystemPromptText = "first message is not system prompt" + toolCallSequenceCode = 11148 + toolCallSequenceText = "tool calls and tool results do not match" // rateLimitCode marks a usage limit whose response carries the absolute // reset timestamp. Cooling down for the generic rate-limit fallback would diff --git a/internal/providers/workbuddy/payload.go b/internal/providers/workbuddy/payload.go index 65ac4d6..4eb82df 100644 --- a/internal/providers/workbuddy/payload.go +++ b/internal/providers/workbuddy/payload.go @@ -18,6 +18,7 @@ func PrepareBody(src []byte) []byte { body["stream"] = true normalizeToolChoice(body) dropEmptyTools(body) + repairToolSequence(body) normalizeEmptyMessageContent(body) ensureLeadingSystem(body) out, err := json.Marshal(body) @@ -43,15 +44,7 @@ func normalizeEmptyMessageContent(body map[string]any) { kept = append(kept, item) continue } - content, exists := message["content"] - empty := !exists || content == nil - if value, ok := content.(string); ok { - empty = strings.TrimSpace(value) == "" - } - if value, ok := content.([]any); ok { - empty = len(value) == 0 - } - if empty && !hasToolCalls(message) { + if messageContentEmpty(message) && !hasToolCalls(message) && !isToolResult(message) { continue } kept = append(kept, item) @@ -59,6 +52,20 @@ func normalizeEmptyMessageContent(body map[string]any) { body["messages"] = kept } +func messageContentEmpty(message map[string]any) bool { + content, exists := message["content"] + if !exists || content == nil { + return true + } + if value, ok := content.(string); ok { + return strings.TrimSpace(value) == "" + } + if value, ok := content.([]any); ok { + return len(value) == 0 + } + return false +} + func hasToolCalls(message map[string]any) bool { raw, ok := message["tool_calls"] if !ok || raw == nil { @@ -70,6 +77,128 @@ func hasToolCalls(message map[string]any) bool { return true } +func isToolResult(message map[string]any) bool { + return messageRole(message) == "tool" || strings.TrimSpace(stringField(message, "tool_call_id")) != "" +} + +func messageRole(message map[string]any) string { + role, _ := message["role"].(string) + return strings.ToLower(strings.TrimSpace(role)) +} + +func stringField(message map[string]any, key string) string { + value, _ := message[key].(string) + return strings.TrimSpace(value) +} + +// repairToolSequence preserves complete tool rounds byte-for-byte at the +// message level and drops only incomplete rounds. Clients that stop a stream +// mid-tool often resend an assistant tool_calls message without all results; +// WorkBuddy then rejects the next turn with code 11148. +func repairToolSequence(body map[string]any) { + raw, ok := body["messages"] + if !ok { + return + } + list, ok := raw.([]any) + if !ok { + return + } + kept := make([]any, 0, len(list)) + for i := 0; i < len(list); { + message, ok := list[i].(map[string]any) + if !ok { + kept = append(kept, list[i]) + i++ + continue + } + if messageRole(message) == "tool" { + i++ + continue + } + if messageRole(message) != "assistant" || !hasToolCalls(message) { + kept = append(kept, message) + i++ + continue + } + calls, _ := message["tool_calls"].([]any) + results := make([]map[string]any, 0) + j := i + 1 + for j < len(list) { + next, ok := list[j].(map[string]any) + if !ok || messageRole(next) != "tool" { + break + } + results = append(results, next) + j++ + } + if !hasCompleteToolRound(calls, results) { + delete(message, "tool_calls") + if !messageContentEmpty(message) { + kept = append(kept, message) + } + i = j + continue + } + kept = append(kept, message) + for _, result := range results { + kept = append(kept, result) + } + i = j + } + body["messages"] = kept +} + +func hasCompleteToolRound(calls []any, results []map[string]any) bool { + if len(calls) == 0 || len(calls) != len(results) { + return false + } + callIDs := make(map[string]struct{}, len(calls)) + for _, raw := range calls { + call, ok := raw.(map[string]any) + if !ok { + return false + } + id := toolCallID(call) + if id == "" { + return false + } + if _, exists := callIDs[id]; exists { + return false + } + callIDs[id] = struct{}{} + } + resultIDs := make(map[string]struct{}, len(results)) + for _, result := range results { + id := toolResultID(result) + if id == "" { + return false + } + if _, exists := callIDs[id]; !exists { + return false + } + if _, exists := resultIDs[id]; exists { + return false + } + resultIDs[id] = struct{}{} + } + return len(callIDs) == len(resultIDs) +} + +func toolCallID(call map[string]any) string { + if id := stringField(call, "id"); id != "" { + return id + } + return stringField(call, "tool_call_id") +} + +func toolResultID(result map[string]any) string { + if id := stringField(result, "tool_call_id"); id != "" { + return id + } + return stringField(result, "id") +} + // ensureLeadingSystem satisfies WorkBuddy Global code 11128 ("first message // is not system prompt"). Drop-system-prompt strips caller identity, which // would otherwise leave a user message first. The placeholder must be non-empty