From 53c90ab58143ac4a4e0060513b023e4afa5fb871 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=94=A1=E5=8F=8A?= <522caiji@gmail.com> Date: Wed, 16 Sep 2026 14:59:54 +0800 Subject: [PATCH] fix(devin): attach tools_diag on MCP configuration denials When Devin rejects a request for MCP configuration reasons, log and append a compact inbound/outbound tools type/name summary so request history can show what Desktop sent versus what we forwarded. --- CHANGELOG.md | 2 + internal/providers/devin/chat.go | 50 ++++++---- internal/providers/devin/devin_test.go | 56 +++++++++++- internal/providers/devin/errors.go | 28 +++++- internal/providers/devin/payload.go | 122 ++++++++++++++++++++++++- 5 files changed, 234 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0291156..dcdfccc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,10 +8,12 @@ Write each change in both `### English` and `### 中文` under `## Unreleased`. ### English - Show consumed points under the Tokens column in request history when a provider reports them; keep writing the value on the request log row, and fall back to the request-detail table for rows that only stored it there +- Temporary Devin diagnostic: when upstream returns an MCP configuration `permission_denied`, append a compact tools type/name summary (`tools_diag`) to the error so request logs can show what Desktop sent versus what was forwarded ### 中文 - 请求历史上游若回报消耗点数，会在 Tokens 列下方以绿色小字展示；继续写入请求日志主表对应字段，并对仅记在详情表的历史行做回退读取 +- 临时诊断：Devin 上游返回 MCP 配置类 `permission_denied` 时，会在错误信息追加精简的 tools type/name 摘要（`tools_diag`），便于从请求日志对照 Desktop 入站与实际上游转发内容 ## 0.5.4 - 2026-09-16 diff --git a/internal/providers/devin/chat.go b/internal/providers/devin/chat.go index f6275e3..f1c73e8 100644 --- a/internal/providers/devin/chat.go +++ b/internal/providers/devin/chat.go @@ -14,12 +14,18 @@ import ( "github.com/caigee-cmd/cli2api/internal/translate" ) +type chatRequestBuild struct { + httpReq *http.Request + originalByAlias map[string]string + toolsDiag string +} + func (c *Client) ChatNonStream(ctx context.Context, accountID string, req translate.ChatRequest) (providers.ChatOutcome, error) { credential, err := c.credential(ctx, accountID) if err != nil { return providers.ChatOutcome{}, err } - httpReq, originalByAlias, err := c.buildChatHTTPRequest(ctx, credential, req) + built, err := c.buildChatHTTPRequest(ctx, credential, req) if err != nil { return providers.ChatOutcome{}, err } @@ -28,16 +34,16 @@ func (c *Client) ChatNonStream(ctx context.Context, accountID string, req transl return providers.ChatOutcome{}, err } client.Timeout = 0 - resp, err := client.Do(httpReq) + resp, err := client.Do(built.httpReq) if err != nil { return providers.ChatOutcome{}, err } defer resp.Body.Close() if resp.StatusCode >= 300 { body, _ := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) - return providers.ChatOutcome{}, classifiedError(resp.StatusCode, string(body)) + return providers.ChatOutcome{}, classifiedErrorWithToolsDiag(resp.StatusCode, string(body), built.toolsDiag) } - aggregate, err := aggregateConnectStream(resp.Body, originalByAlias) + aggregate, err := aggregateConnectStream(resp.Body, built.originalByAlias, built.toolsDiag) if err != nil { return providers.ChatOutcome{}, err } @@ -49,7 +55,7 @@ func (c *Client) ChatStream(ctx context.Context, accountID string, req translate if err != nil { return nil, err } - httpReq, originalByAlias, err := c.buildChatHTTPRequest(ctx, credential, req) + built, err := c.buildChatHTTPRequest(ctx, credential, req) if err != nil { return nil, err } @@ -58,19 +64,19 @@ func (c *Client) ChatStream(ctx context.Context, accountID string, req translate return nil, err } client.Timeout = 0 - resp, err := client.Do(httpReq) + resp, err := client.Do(built.httpReq) if err != nil { return nil, err } if resp.StatusCode >= 300 { body, _ := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) resp.Body.Close() - return nil, classifiedError(resp.StatusCode, string(body)) + return nil, classifiedErrorWithToolsDiag(resp.StatusCode, string(body), built.toolsDiag) } - return rewriteConnectStream(resp, firstNonEmpty(req.Model, "devin"), originalByAlias) + return rewriteConnectStream(resp, firstNonEmpty(req.Model, "devin"), built.originalByAlias, built.toolsDiag) } -func (c *Client) buildChatHTTPRequest(ctx context.Context, credential Credential, req translate.ChatRequest) (*http.Request, map[string]string, error) { +func (c *Client) buildChatHTTPRequest(ctx context.Context, credential Credential, req translate.ChatRequest) (chatRequestBuild, error) { payload := BuildChatPayload(req, currentLevels()) proto := BuildGetChatMessageRequest( credential.SessionToken, @@ -88,7 +94,7 @@ func (c *Client) buildChatHTTPRequest(ctx context.Context, credential Credential endpoint := strings.TrimRight(firstNonEmpty(credential.BaseURL, c.serverBase, ServerBase), "/") + PathGetChatMessage httpReq, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewReader(body)) if err != nil { - return nil, nil, err + return chatRequestBuild{}, err } httpReq.Header.Set("Authorization", BasicAuthHeader(credential.SessionToken)) httpReq.Header.Set("Content-Type", ContentTypeConnectProto) @@ -96,7 +102,11 @@ func (c *Client) buildChatHTTPRequest(ctx context.Context, credential Credential httpReq.Header.Set("Accept", "*/*") httpReq.Header.Set("Sentry-Trace", GenerateSentryTrace()) httpReq.Header["User-Agent"] = []string{""} - return httpReq, payload.OriginalByAlias, nil + return chatRequestBuild{ + httpReq: httpReq, + originalByAlias: payload.OriginalByAlias, + toolsDiag: payload.ToolsDiag, + }, nil } type aggregateResult struct { @@ -109,7 +119,7 @@ type aggregateResult struct { CompletionTokens int } -func aggregateConnectStream(r io.Reader, originalByAlias map[string]string) (aggregateResult, error) { +func aggregateConnectStream(r io.Reader, originalByAlias map[string]string, toolsDiag string) (aggregateResult, error) { var out aggregateResult out.FinishReason = "stop" toolAcc := map[int]*ToolCallDelta{} @@ -125,7 +135,7 @@ func aggregateConnectStream(r io.Reader, originalByAlias map[string]string) (agg if flag&ConnectFlagEndStream != 0 { sawEOS = true if status, trailerErr := ParseTrailerError(payload); trailerErr != nil { - return out, classifiedError(status, trailerErr.Error()) + return out, classifiedErrorWithToolsDiag(status, trailerErr.Error(), toolsDiag) } break } @@ -176,7 +186,7 @@ func aggregateConnectStream(r io.Reader, originalByAlias map[string]string) (agg } } if !sawEOS { - return out, classifiedError(502, "devin stream truncated: missing EOS trailer") + return out, classifiedErrorWithToolsDiag(502, "devin stream truncated: missing EOS trailer", toolsDiag) } if len(toolAcc) > 0 { out.FinishReason = "tool_calls" @@ -225,7 +235,7 @@ func outcomeFromAggregate(aggregate aggregateResult, fallbackModel string) provi return out } -func rewriteConnectStream(upstream *http.Response, model string, originalByAlias map[string]string) (*http.Response, error) { +func rewriteConnectStream(upstream *http.Response, model string, originalByAlias map[string]string, toolsDiag string) (*http.Response, error) { pr, pw := io.Pipe() go func() { defer upstream.Body.Close() @@ -273,7 +283,7 @@ func rewriteConnectStream(upstream *http.Response, model string, originalByAlias if flag&ConnectFlagEndStream != 0 { sawEOS = true if status, trailerErr := ParseTrailerError(payload); trailerErr != nil { - _ = pw.CloseWithError(classifiedError(status, trailerErr.Error())) + _ = pw.CloseWithError(classifiedErrorWithToolsDiag(status, trailerErr.Error(), toolsDiag)) return } break @@ -347,10 +357,10 @@ func rewriteConnectStream(upstream *http.Response, model string, originalByAlias } } } - if !sawEOS { - _ = pw.CloseWithError(classifiedError(502, "devin stream truncated: missing EOS trailer")) - return - } + if !sawEOS { + _ = pw.CloseWithError(classifiedErrorWithToolsDiag(502, "devin stream truncated: missing EOS trailer", toolsDiag)) + return + } finish := "stop" if len(toolAcc) > 0 { finish = "tool_calls" diff --git a/internal/providers/devin/devin_test.go b/internal/providers/devin/devin_test.go index d85194d..9607d72 100644 --- a/internal/providers/devin/devin_test.go +++ b/internal/providers/devin/devin_test.go @@ -373,7 +373,7 @@ func TestAggregateConnectStreamMissingEOS(t *testing.T) { textFrame = AppendTag(textFrame, 3, BytesType) textFrame = AppendString(textFrame, "orphan") framed := WrapConnectEnvelope(textFrame) - _, err := aggregateConnectStream(bytes.NewReader(framed), nil) + _, err := aggregateConnectStream(bytes.NewReader(framed), nil, "") if err == nil { t.Fatal("expected missing EOS error") } @@ -542,6 +542,60 @@ func TestClassifyMCPConfigPermissionDenied(t *testing.T) { if providerErr.Failover == nil || *providerErr.Failover { t.Fatalf("failover=%v want false", providerErr.Failover) } + + diag := "in=[namespace:mcp__computer-use,ns.function:left_click] out(1)=[mcp_computer_use_left_click]" + err = classifiedErrorWithToolsDiag(403, body, diag) + if !errors.As(err, &providerErr) { + t.Fatalf("classifiedErrorWithToolsDiag type=%T", err) + } + if !strings.Contains(providerErr.Message, "tools_diag="+diag) { + t.Fatalf("message missing tools_diag: %s", providerErr.Message) + } + if providerErr.Kind != accounts.KindInvalidRequest || providerErr.Cooldown != 0 { + t.Fatalf("diag classify=%+v", providerErr) + } +} + +func TestBuildToolsDiagSummarizesInboundAndOutbound(t *testing.T) { + raw := json.RawMessage(`[ + {"type":"namespace","name":"mcp__computer-use","tools":[ + {"type":"function","name":"left_click","parameters":{"type":"object"}}, + {"type":"function","function":{"name":"mcp__computer-use__type","parameters":{"type":"object"}}} + ]}, + {"type":"mcp","server_label":"browser"}, + {"type":"web_search"}, + {"type":"custom","name":"weird_shell"}, + {"type":"function","function":{"name":"lookup","parameters":{"type":"object"}}} + ]`) + payload := BuildChatPayload(translate.ChatRequest{ + Model: "swe-2", + Messages: []translate.ChatMessage{{Role: "user", Content: "hi"}}, + Tools: raw, + }, nil) + if !strings.Contains(payload.ToolsDiag, "in=[") { + t.Fatalf("missing inbound summary: %s", payload.ToolsDiag) + } + if !strings.Contains(payload.ToolsDiag, "namespace:mcp__computer-use") { + t.Fatalf("missing namespace entry: %s", payload.ToolsDiag) + } + if !strings.Contains(payload.ToolsDiag, "ns.function:left_click") { + t.Fatalf("missing nested function: %s", payload.ToolsDiag) + } + if !strings.Contains(payload.ToolsDiag, "mcp:browser") { + t.Fatalf("missing mcp shell: %s", payload.ToolsDiag) + } + if !strings.Contains(payload.ToolsDiag, "web_search") { + t.Fatalf("missing web_search: %s", payload.ToolsDiag) + } + if !strings.Contains(payload.ToolsDiag, "custom:weird_shell") { + t.Fatalf("missing custom type: %s", payload.ToolsDiag) + } + if !strings.Contains(payload.ToolsDiag, "out(") || !strings.Contains(payload.ToolsDiag, "lookup") { + t.Fatalf("missing outbound summary: %s", payload.ToolsDiag) + } + if strings.Contains(payload.ToolsDiag, `"parameters"`) || strings.Contains(payload.ToolsDiag, "description") { + t.Fatalf("diag leaked schema/description: %s", payload.ToolsDiag) + } } func TestParseToolsAliasesMCPNamespace(t *testing.T) { diff --git a/internal/providers/devin/errors.go b/internal/providers/devin/errors.go index 5b57804..6ce44da 100644 --- a/internal/providers/devin/errors.go +++ b/internal/providers/devin/errors.go @@ -1,6 +1,7 @@ package devin import ( + "log" "regexp" "strings" "time" @@ -94,6 +95,10 @@ func Classify(status int, body string) providers.ClassifiedError { } func classifiedError(status int, body string) error { + return classifiedErrorWithToolsDiag(status, body, "") +} + +func classifiedErrorWithToolsDiag(status int, body, toolsDiag string) error { classified := Classify(status, body) if classified.Kind == "" { classified = providers.ClassifiedError{ @@ -102,16 +107,37 @@ func classifiedError(status int, body string) error { Message: firstNonEmpty(redactSecrets(strings.TrimSpace(body)), "upstream error"), } } + message := classified.Message + if toolsDiag != "" && isDevinMCPConfigDenial(strings.ToLower(message)+" "+strings.ToLower(body)) { + log.Printf("devin mcp configuration denial tools_diag=%s", toolsDiag) + message = appendToolsDiag(message, toolsDiag) + } failover := classified.Kind != accounts.KindInvalidRequest return &providers.Error{ Kind: classified.Kind, Status: classified.Status, - Message: classified.Message, + Message: message, Cooldown: classifiedCooldown(classified.Kind), Failover: &failover, } } +func appendToolsDiag(message, toolsDiag string) string { + message = strings.TrimSpace(message) + toolsDiag = strings.TrimSpace(toolsDiag) + if toolsDiag == "" { + return message + } + suffix := "tools_diag=" + toolsDiag + if message == "" { + return suffix + } + if strings.Contains(message, "tools_diag=") { + return message + } + return message + " | " + suffix +} + func classifiedCooldown(kind string) time.Duration { switch kind { case accounts.KindQuota: diff --git a/internal/providers/devin/payload.go b/internal/providers/devin/payload.go index 8bd572d..1a665b6 100644 --- a/internal/providers/devin/payload.go +++ b/internal/providers/devin/payload.go @@ -4,13 +4,19 @@ import ( "crypto/sha256" "encoding/hex" "encoding/json" + "fmt" "strconv" "strings" "github.com/caigee-cmd/cli2api/internal/translate" ) -const maxDevinToolAliasLen = 64 +const ( + maxDevinToolAliasLen = 64 + maxDevinToolsDiagLen = 1800 + maxDevinToolDiagNameLen = 96 + maxDevinNamespaceNestLen = 12 +) // ChatPayload is the normalized Devin Interactions request. type ChatPayload struct { @@ -23,6 +29,10 @@ type ChatPayload struct { Effort string Budget int OriginalByAlias map[string]string + // ToolsDiag is a compact inbound/outbound tools type+name summary for + // temporary MCP configuration denial debugging. It never includes + // descriptions or parameter schemas. + ToolsDiag string } func BuildChatPayload(req translate.ChatRequest, catalogLevels map[string][]string) ChatPayload { @@ -63,17 +73,19 @@ func BuildChatPayload(req translate.ChatRequest, catalogLevels map[string][]stri maxTokens := parseMaxTokens(req) temp := parseTemperature(req) modelUID := ResolveChatModelUID(req.Model, effort, budget, catalogLevels) + tools := parseTools(req.Tools, aliases) return ChatPayload{ System: system, Prompts: prompts, - Tools: parseTools(req.Tools, aliases), + Tools: tools, Temperature: temp, MaxTokens: maxTokens, ModelUID: modelUID, Effort: effort, Budget: budget, OriginalByAlias: aliases.originalByAlias, + ToolsDiag: buildToolsDiag(req.Tools, tools), } } @@ -374,6 +386,112 @@ func restoreToolName(name string, originalByAlias map[string]string) string { return name } +func buildToolsDiag(raw json.RawMessage, outbound []Tool) string { + inbound := summarizeInboundTools(raw) + outNames := make([]string, 0, len(outbound)) + for _, tool := range outbound { + if name := truncateDiagName(tool.Name); name != "" { + outNames = append(outNames, name) + } + } + parts := make([]string, 0, 2) + if inbound != "" { + parts = append(parts, "in="+inbound) + } else if len(raw) > 0 { + parts = append(parts, "in=") + } else { + parts = append(parts, "in=") + } + if len(outNames) == 0 { + parts = append(parts, "out=") + } else { + parts = append(parts, fmt.Sprintf("out(%d)=[%s]", len(outNames), strings.Join(outNames, ","))) + } + return truncateDiag(strings.Join(parts, " ")) +} + +func summarizeInboundTools(raw json.RawMessage) string { + if len(raw) == 0 { + return "" + } + var items []json.RawMessage + if json.Unmarshal(raw, &items) != nil { + return "" + } + entries := make([]string, 0, len(items)) + for _, item := range items { + entries = append(entries, summarizeInboundToolItem(item)...) + } + if len(entries) == 0 { + return "[]" + } + return fmt.Sprintf("[%s]", strings.Join(entries, ",")) +} + +func summarizeInboundToolItem(raw json.RawMessage) []string { + var probe struct { + Type string `json:"type"` + Name string `json:"name"` + Function struct { + Name string `json:"name"` + } `json:"function"` + ServerLabel string `json:"server_label"` + Tools []json.RawMessage `json:"tools"` + } + if json.Unmarshal(raw, &probe) != nil { + return []string{""} + } + typ := strings.ToLower(strings.TrimSpace(probe.Type)) + if typ == "" { + typ = "function" + } + name := firstNonEmpty(probe.Function.Name, probe.Name, probe.ServerLabel) + entry := typ + if trimmed := truncateDiagName(name); trimmed != "" { + entry += ":" + trimmed + } + out := []string{entry} + if typ == "namespace" { + limit := len(probe.Tools) + if limit > maxDevinNamespaceNestLen { + limit = maxDevinNamespaceNestLen + } + for i := 0; i < limit; i++ { + for _, nested := range summarizeInboundToolItem(probe.Tools[i]) { + out = append(out, "ns."+nested) + } + } + if len(probe.Tools) > maxDevinNamespaceNestLen { + out = append(out, fmt.Sprintf("ns.<+%d>", len(probe.Tools)-maxDevinNamespaceNestLen)) + } + } + return out +} + +func truncateDiagName(name string) string { + name = strings.TrimSpace(name) + if name == "" { + return "" + } + runes := []rune(name) + if len(runes) <= maxDevinToolDiagNameLen { + return name + } + return string(runes[:maxDevinToolDiagNameLen-1]) + "…" +} + +func truncateDiag(text string) string { + text = strings.TrimSpace(text) + if text == "" { + return "" + } + runes := []rune(text) + if len(runes) <= maxDevinToolsDiagLen { + return text + } + return string(runes[:maxDevinToolsDiagLen-1]) + "…" +} + func extractReasoning(msg translate.ChatMessage) string { // ChatMessage has no dedicated reasoning field; try content parts with type=thinking. if parts, ok := msg.Content.([]any); ok {