fix
This commit is contained in:
@@ -35,8 +35,8 @@ func TestResolveCompleterForRole_unsetFallsBackToEnv(t *testing.T) {
|
|||||||
if !ok || oc == nil || !oc.Enabled() {
|
if !ok || oc == nil || !oc.Enabled() {
|
||||||
t.Fatalf("expected enabled OpenAIClient, got %T", c)
|
t.Fatalf("expected enabled OpenAIClient, got %T", c)
|
||||||
}
|
}
|
||||||
if label != ModeInternalLabel {
|
if label != ModeCustomLabel {
|
||||||
t.Fatalf("label=%q", label)
|
t.Fatalf("label=%q want %q (env bootstrap is custom)", label, ModeCustomLabel)
|
||||||
}
|
}
|
||||||
if byok {
|
if byok {
|
||||||
t.Fatal("env fallback must not be BYOK")
|
t.Fatal("env fallback must not be BYOK")
|
||||||
@@ -97,8 +97,8 @@ func TestResolveCompleterForRole_platformProcessingRole(t *testing.T) {
|
|||||||
if oc.APIKey != "sk-plat-processing" || oc.Model != "plat-model" {
|
if oc.APIKey != "sk-plat-processing" || oc.Model != "plat-model" {
|
||||||
t.Fatalf("key=%q model=%q", oc.APIKey, oc.Model)
|
t.Fatalf("key=%q model=%q", oc.APIKey, oc.Model)
|
||||||
}
|
}
|
||||||
if label != ModeInternalLabel || byok {
|
if label != ModeCustomLabel || byok {
|
||||||
t.Fatalf("label=%q byok=%v", label, byok)
|
t.Fatalf("label=%q byok=%v want custom (loopback platform base)", label, byok)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -389,9 +389,14 @@ func (s *Service) TestPlatformRole(ctx context.Context, role string) (map[string
|
|||||||
out["message"] = "AI role is not configured (or disabled) in admin platform settings"
|
out["message"] = "AI role is not configured (or disabled) in admin platform settings"
|
||||||
return out, nil
|
return out, nil
|
||||||
}
|
}
|
||||||
if _, err := completer.Complete(ctx, "Reply with exactly: ok", "ping"); err != nil {
|
if err := probeCompleter(ctx, completer); err != nil {
|
||||||
out["status"] = "failed"
|
out["status"] = "failed"
|
||||||
out["message"] = "connection failed — check provider, key, base URL, and model"
|
// Prefer classified probe detail (no secrets / model-id dumps) over a generic blur.
|
||||||
|
msg := processing.TruncateError(err)
|
||||||
|
if msg == "" || msg == "processing_failed" || msg == "provider error (details redacted)" {
|
||||||
|
msg = "connection failed — check provider, key, base URL, and model"
|
||||||
|
}
|
||||||
|
out["message"] = msg
|
||||||
return out, err
|
return out, err
|
||||||
}
|
}
|
||||||
out["status"] = "ok"
|
out["status"] = "ok"
|
||||||
@@ -399,6 +404,21 @@ func (s *Service) TestPlatformRole(ctx context.Context, role string) (map[string
|
|||||||
return out, nil
|
return out, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// probeCompleter runs a minimal chat completion with GPT-5.6-safe options
|
||||||
|
// (max_completion_tokens + reasoning_effort=none, no custom temperature).
|
||||||
|
func probeCompleter(ctx context.Context, completer processing.Completer) error {
|
||||||
|
if completer == nil {
|
||||||
|
return ErrNotConfigured
|
||||||
|
}
|
||||||
|
opts := processing.ProbeCompleteOptions()
|
||||||
|
if co, ok := completer.(processing.CompleterWithOptions); ok {
|
||||||
|
_, err := co.CompleteWithOptions(ctx, "Reply with exactly: ok", "ping", opts)
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
_, err := completer.Complete(ctx, "Reply with exactly: ok", "ping")
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
// TestConnection sends a minimal chat completion and records last_test_*.
|
// TestConnection sends a minimal chat completion and records last_test_*.
|
||||||
func (s *Service) TestConnection(ctx context.Context, companyID uuid.UUID) (map[string]any, error) {
|
func (s *Service) TestConnection(ctx context.Context, companyID uuid.UUID) (map[string]any, error) {
|
||||||
ctx, cancel := context.WithTimeout(ctx, aiProbeTimeout)
|
ctx, cancel := context.WithTimeout(ctx, aiProbeTimeout)
|
||||||
@@ -422,12 +442,10 @@ func (s *Service) TestConnection(ctx context.Context, companyID uuid.UUID) (map[
|
|||||||
WHERE company_id = $1`, companyID, status)
|
WHERE company_id = $1`, companyID, status)
|
||||||
return map[string]any{"status": status, "message": message, "mode": resolved.ModeLabel}, ErrNotConfigured
|
return map[string]any{"status": status, "message": message, "mode": resolved.ModeLabel}, ErrNotConfigured
|
||||||
}
|
}
|
||||||
_, err = resolved.Completer.Complete(ctx, "Reply with exactly: ok", "ping")
|
if err := probeCompleter(ctx, resolved.Completer); err != nil {
|
||||||
if err != nil {
|
|
||||||
status = "failed"
|
status = "failed"
|
||||||
// TruncateError classifies transport/auth failures without leaking secrets.
|
|
||||||
message = processing.TruncateError(err)
|
message = processing.TruncateError(err)
|
||||||
if message == "" || message == "provider error (details redacted)" {
|
if message == "" || message == "processing_failed" || message == "provider error (details redacted)" {
|
||||||
message = "connection failed — check provider, key, base URL, and model"
|
message = "connection failed — check provider, key, base URL, and model"
|
||||||
}
|
}
|
||||||
_, _ = s.Pool.Exec(ctx, `
|
_, _ = s.Pool.Exec(ctx, `
|
||||||
|
|||||||
@@ -243,10 +243,7 @@ type chatResponse struct {
|
|||||||
CompletionTokens int `json:"completion_tokens"`
|
CompletionTokens int `json:"completion_tokens"`
|
||||||
TotalTokens int `json:"total_tokens"`
|
TotalTokens int `json:"total_tokens"`
|
||||||
} `json:"usage"`
|
} `json:"usage"`
|
||||||
Error *struct {
|
Error *openAIErrorBody `json:"error"`
|
||||||
Message string `json:"message"`
|
|
||||||
Type string `json:"type"`
|
|
||||||
} `json:"error"`
|
|
||||||
}
|
}
|
||||||
|
|
||||||
var (
|
var (
|
||||||
@@ -466,8 +463,13 @@ func (c *OpenAIClient) doEmbed(ctx context.Context, texts []string) ([][]float32
|
|||||||
}
|
}
|
||||||
|
|
||||||
func (c *OpenAIClient) doComplete(ctx context.Context, system, user string, temperature float64, maxTokens int, reasoningEffort string) (Completion, bool, error) {
|
func (c *OpenAIClient) doComplete(ctx context.Context, system, user string, temperature float64, maxTokens int, reasoningEffort string) (Completion, bool, error) {
|
||||||
|
sysRole := "system"
|
||||||
|
if openAIReasoningChatModel(c.Model) {
|
||||||
|
// GPT-5 family prefers developer over system on Chat Completions.
|
||||||
|
sysRole = "developer"
|
||||||
|
}
|
||||||
reqBody := buildChatCompletionBody(c.Model, []chatMessage{
|
reqBody := buildChatCompletionBody(c.Model, []chatMessage{
|
||||||
{Role: "system", Content: system},
|
{Role: sysRole, Content: system},
|
||||||
{Role: "user", Content: user},
|
{Role: "user", Content: user},
|
||||||
}, temperature, maxTokens, reasoningEffort)
|
}, temperature, maxTokens, reasoningEffort)
|
||||||
body, err := json.Marshal(reqBody)
|
body, err := json.Marshal(reqBody)
|
||||||
@@ -502,19 +504,13 @@ func (c *OpenAIClient) doComplete(ctx context.Context, system, user string, temp
|
|||||||
return Completion{}, false, decErr
|
return Completion{}, false, decErr
|
||||||
}
|
}
|
||||||
if res.StatusCode == http.StatusTooManyRequests || res.StatusCode >= 500 {
|
if res.StatusCode == http.StatusTooManyRequests || res.StatusCode >= 500 {
|
||||||
msg := "rate limited or server error"
|
msg := formatOpenAIHTTPError(res.StatusCode, parsed.Error)
|
||||||
if parsed.Error != nil && parsed.Error.Message != "" {
|
|
||||||
msg = TruncateError(errors.New(parsed.Error.Message))
|
|
||||||
}
|
|
||||||
httpErr := errors.New(msg)
|
httpErr := errors.New(msg)
|
||||||
stopWait(httpErr)
|
stopWait(httpErr)
|
||||||
return Completion{}, true, httpErr
|
return Completion{}, true, httpErr
|
||||||
}
|
}
|
||||||
if res.StatusCode >= 400 {
|
if res.StatusCode >= 400 {
|
||||||
msg := fmt.Sprintf("openai http %d", res.StatusCode)
|
msg := formatOpenAIHTTPError(res.StatusCode, parsed.Error)
|
||||||
if parsed.Error != nil && parsed.Error.Message != "" {
|
|
||||||
msg = TruncateError(errors.New(parsed.Error.Message))
|
|
||||||
}
|
|
||||||
httpErr := errors.New(msg)
|
httpErr := errors.New(msg)
|
||||||
stopWait(httpErr)
|
stopWait(httpErr)
|
||||||
return Completion{}, false, httpErr
|
return Completion{}, false, httpErr
|
||||||
|
|||||||
@@ -0,0 +1,110 @@
|
|||||||
|
package processing
|
||||||
|
|
||||||
|
import (
|
||||||
|
"fmt"
|
||||||
|
"strings"
|
||||||
|
)
|
||||||
|
|
||||||
|
// openAIErrorBody is the error object returned by OpenAI-compatible APIs.
|
||||||
|
type openAIErrorBody struct {
|
||||||
|
Message string `json:"message"`
|
||||||
|
Type string `json:"type"`
|
||||||
|
Code any `json:"code"` // string or number depending on provider
|
||||||
|
Param string `json:"param"`
|
||||||
|
}
|
||||||
|
|
||||||
|
func (e *openAIErrorBody) codeString() string {
|
||||||
|
if e == nil {
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
switch v := e.Code.(type) {
|
||||||
|
case string:
|
||||||
|
return strings.TrimSpace(v)
|
||||||
|
case float64:
|
||||||
|
if v == float64(int64(v)) {
|
||||||
|
return fmt.Sprintf("%d", int64(v))
|
||||||
|
}
|
||||||
|
return fmt.Sprintf("%g", v)
|
||||||
|
case int:
|
||||||
|
return fmt.Sprintf("%d", v)
|
||||||
|
default:
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// formatOpenAIHTTPError turns an upstream HTTP error into a short, secret-safe
|
||||||
|
// operator message. Intentionally avoids embedding model ids (gpt-…) so
|
||||||
|
// TruncateError / mapPublicErrorCode do not collapse useful validation text to
|
||||||
|
// "processing_failed".
|
||||||
|
func formatOpenAIHTTPError(status int, errObj *openAIErrorBody) string {
|
||||||
|
msg := ""
|
||||||
|
code := ""
|
||||||
|
param := ""
|
||||||
|
typ := ""
|
||||||
|
if errObj != nil {
|
||||||
|
msg = strings.TrimSpace(errObj.Message)
|
||||||
|
code = errObj.codeString()
|
||||||
|
param = strings.TrimSpace(errObj.Param)
|
||||||
|
typ = strings.TrimSpace(errObj.Type)
|
||||||
|
}
|
||||||
|
lower := strings.ToLower(msg)
|
||||||
|
codeLower := strings.ToLower(code)
|
||||||
|
paramLower := strings.ToLower(param)
|
||||||
|
|
||||||
|
switch {
|
||||||
|
case status == 401 || codeLower == "invalid_api_key" ||
|
||||||
|
strings.Contains(lower, "invalid api key") ||
|
||||||
|
strings.Contains(lower, "incorrect api key"):
|
||||||
|
return "AI provider rejected the API key"
|
||||||
|
case status == 403:
|
||||||
|
return "AI provider forbidden the request"
|
||||||
|
case status == 429 || codeLower == "rate_limit_exceeded":
|
||||||
|
return "AI provider rate limited — retry later"
|
||||||
|
case codeLower == "insufficient_quota" || strings.Contains(lower, "insufficient_quota") ||
|
||||||
|
strings.Contains(lower, "exceeded your current quota"):
|
||||||
|
return "OpenAI quota exceeded — check billing"
|
||||||
|
case strings.Contains(lower, "does not exist") || strings.Contains(lower, "do not have access") ||
|
||||||
|
codeLower == "model_not_found":
|
||||||
|
return "model not found or API key lacks access — check model name and project permissions"
|
||||||
|
case strings.Contains(lower, "max_tokens") && strings.Contains(lower, "max_completion"):
|
||||||
|
return "model requires completion-token budget (legacy token cap unsupported)"
|
||||||
|
case paramLower == "temperature" || strings.Contains(lower, "temperature"):
|
||||||
|
return "model rejects custom temperature — omit temperature for reasoning chat models"
|
||||||
|
case paramLower == "max_tokens" || (strings.Contains(lower, "unsupported parameter") && strings.Contains(lower, "max_tokens")):
|
||||||
|
return "model rejects legacy token cap — use completion-token budget"
|
||||||
|
case paramLower == "reasoning_effort" || strings.Contains(lower, "reasoning_effort"):
|
||||||
|
return "model rejected reasoning effort — try none/low or omit"
|
||||||
|
case strings.Contains(lower, "unsupported parameter") || strings.Contains(lower, "unsupported value"):
|
||||||
|
if param != "" {
|
||||||
|
return fmt.Sprintf("unsupported chat parameter %q", param)
|
||||||
|
}
|
||||||
|
return "unsupported chat parameter for this model"
|
||||||
|
case status >= 500:
|
||||||
|
return "AI provider temporarily unavailable (server error)"
|
||||||
|
}
|
||||||
|
|
||||||
|
// Prefer structured fields over raw message (raw often embeds model ids / urls).
|
||||||
|
if code != "" && param != "" {
|
||||||
|
return fmt.Sprintf("provider http %d (%s param=%s)", status, code, param)
|
||||||
|
}
|
||||||
|
if code != "" {
|
||||||
|
return fmt.Sprintf("provider http %d (%s)", status, code)
|
||||||
|
}
|
||||||
|
if typ != "" {
|
||||||
|
return fmt.Sprintf("provider http %d (%s)", status, typ)
|
||||||
|
}
|
||||||
|
if status > 0 {
|
||||||
|
return fmt.Sprintf("provider http %d", status)
|
||||||
|
}
|
||||||
|
return "provider request failed"
|
||||||
|
}
|
||||||
|
|
||||||
|
// ProbeCompleteOptions is the Chat Completions budget for admin connection tests.
|
||||||
|
// Explicit max_completion_tokens + reasoning_effort=none avoids GPT-5.6 defaults
|
||||||
|
// spending the budget on hidden reasoning for a one-token "ok" reply.
|
||||||
|
func ProbeCompleteOptions() CompleteOptions {
|
||||||
|
return CompleteOptions{
|
||||||
|
MaxTokens: 64,
|
||||||
|
ReasoningEffort: "none",
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,90 @@
|
|||||||
|
package processing
|
||||||
|
|
||||||
|
import (
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestFormatOpenAIHTTPError_preservesUsefulHints(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
cases := []struct {
|
||||||
|
status int
|
||||||
|
err *openAIErrorBody
|
||||||
|
want string
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
status: 400,
|
||||||
|
err: &openAIErrorBody{
|
||||||
|
Message: "Unsupported parameter: 'temperature' does not support 0.2 with this model. Only the default (1) value is supported.",
|
||||||
|
Type: "invalid_request_error",
|
||||||
|
Code: "unsupported_value",
|
||||||
|
Param: "temperature",
|
||||||
|
},
|
||||||
|
want: "model rejects custom temperature",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
status: 400,
|
||||||
|
err: &openAIErrorBody{
|
||||||
|
Message: "Unsupported parameter: 'max_tokens' is not supported with this model. Use 'max_completion_tokens' instead.",
|
||||||
|
Param: "max_tokens",
|
||||||
|
},
|
||||||
|
want: "model requires completion-token budget",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
status: 404,
|
||||||
|
err: &openAIErrorBody{
|
||||||
|
Message: "The model `gpt-5.6-luna` does not exist or you do not have access to it.",
|
||||||
|
Code: "model_not_found",
|
||||||
|
},
|
||||||
|
want: "model not found or API key lacks access",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
status: 400,
|
||||||
|
err: &openAIErrorBody{
|
||||||
|
Message: "Unsupported parameter: 'max_tokens' is not supported with this model.",
|
||||||
|
Param: "max_tokens",
|
||||||
|
Code: "unsupported_parameter",
|
||||||
|
},
|
||||||
|
want: "model rejects legacy token cap",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
status: 401,
|
||||||
|
err: &openAIErrorBody{
|
||||||
|
Message: "Incorrect API key provided: sk-proj-REDACTED",
|
||||||
|
Code: "invalid_api_key",
|
||||||
|
},
|
||||||
|
want: "AI provider rejected the API key",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
status: 400,
|
||||||
|
err: &openAIErrorBody{
|
||||||
|
Message: "something obscure mentioning gpt-5.6-luna",
|
||||||
|
Type: "invalid_request_error",
|
||||||
|
Code: "invalid_request_error",
|
||||||
|
Param: "messages",
|
||||||
|
},
|
||||||
|
want: "provider http 400 (invalid_request_error param=messages)",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
for _, tc := range cases {
|
||||||
|
got := formatOpenAIHTTPError(tc.status, tc.err)
|
||||||
|
if !strings.Contains(strings.ToLower(got), strings.ToLower(tc.want)) {
|
||||||
|
t.Fatalf("status=%d got=%q want contains %q", tc.status, got, tc.want)
|
||||||
|
}
|
||||||
|
if strings.Contains(got, "sk-") || strings.Contains(got, "gpt-5.6-luna") {
|
||||||
|
t.Fatalf("leaked secret or model id: %q", got)
|
||||||
|
}
|
||||||
|
safe := TruncateError(errString(got))
|
||||||
|
if safe == "processing_failed" {
|
||||||
|
t.Fatalf("TruncateError wiped useful hint: in=%q", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestProbeCompleteOptions(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
opts := ProbeCompleteOptions()
|
||||||
|
if opts.MaxTokens != 64 || opts.ReasoningEffort != "none" {
|
||||||
|
t.Fatalf("%+v", opts)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -184,6 +184,14 @@ func TestOpenAIClient_doComplete_gpt56LunaUsesMaxCompletionTokens(t *testing.T)
|
|||||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
var req map[string]any
|
var req map[string]any
|
||||||
_ = json.NewDecoder(r.Body).Decode(&req)
|
_ = json.NewDecoder(r.Body).Decode(&req)
|
||||||
|
msgs, _ := req["messages"].([]any)
|
||||||
|
if len(msgs) < 1 {
|
||||||
|
t.Errorf("expected messages")
|
||||||
|
} else if m0, ok := msgs[0].(map[string]any); ok {
|
||||||
|
if m0["role"] != "developer" {
|
||||||
|
t.Errorf("system role=%v want developer for gpt-5.6", m0["role"])
|
||||||
|
}
|
||||||
|
}
|
||||||
if _, ok := req["temperature"]; ok {
|
if _, ok := req["temperature"]; ok {
|
||||||
t.Errorf("temperature must be omitted for gpt-5.6-luna")
|
t.Errorf("temperature must be omitted for gpt-5.6-luna")
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -232,17 +232,26 @@ func classifyProviderError(msg string) string {
|
|||||||
strings.Contains(lower, "http 401"),
|
strings.Contains(lower, "http 401"),
|
||||||
strings.Contains(lower, "invalid api key"),
|
strings.Contains(lower, "invalid api key"),
|
||||||
strings.Contains(lower, "incorrect api key"),
|
strings.Contains(lower, "incorrect api key"),
|
||||||
strings.Contains(lower, "invalid_api_key"):
|
strings.Contains(lower, "invalid_api_key"),
|
||||||
|
strings.Contains(lower, "rejected the api key"):
|
||||||
return "AI provider rejected the API key"
|
return "AI provider rejected the API key"
|
||||||
case strings.Contains(lower, "http 403"),
|
case strings.Contains(lower, "http 403"),
|
||||||
lower == "forbidden":
|
lower == "forbidden",
|
||||||
|
strings.Contains(lower, "forbidden the request"):
|
||||||
return "AI provider forbidden the request"
|
return "AI provider forbidden the request"
|
||||||
case strings.Contains(lower, "http 429"),
|
|
||||||
strings.Contains(lower, "too many requests"),
|
|
||||||
lower == "rate limited":
|
|
||||||
return "AI provider rate limited — retry later"
|
|
||||||
case lower == "rate limited or server error":
|
case lower == "rate limited or server error":
|
||||||
return "AI provider temporarily unavailable (rate limited or server error)"
|
return "AI provider temporarily unavailable (rate limited or server error)"
|
||||||
|
case strings.Contains(lower, "http 429"),
|
||||||
|
strings.Contains(lower, "too many requests"),
|
||||||
|
lower == "rate limited",
|
||||||
|
strings.Contains(lower, "rate limited —"),
|
||||||
|
strings.Contains(lower, "rate limited -"):
|
||||||
|
return "AI provider rate limited — retry later"
|
||||||
|
case strings.Contains(lower, "quota exceeded"):
|
||||||
|
return "OpenAI quota exceeded — check billing"
|
||||||
|
case strings.Contains(lower, "model not found"),
|
||||||
|
strings.Contains(lower, "lacks access"):
|
||||||
|
return "model not found or API key lacks access — check model name and project permissions"
|
||||||
}
|
}
|
||||||
return ""
|
return ""
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user