From 103493206e9a3ae49a1f20adfd0fa20159851b2b Mon Sep 17 00:00:00 2001 From: Kayvan Sylvan Date: Mon, 23 Feb 2026 14:29:25 -0800 Subject: [PATCH] fix: Address copilot review comments --- docs/Azure-AI-Gateway.md | 10 +++--- internal/i18n/locales/de.json | 1 + internal/i18n/locales/en.json | 1 + internal/i18n/locales/es.json | 1 + internal/i18n/locales/fa.json | 1 + internal/i18n/locales/fr.json | 1 + internal/i18n/locales/it.json | 1 + internal/i18n/locales/ja.json | 1 + internal/i18n/locales/pt-BR.json | 1 + internal/i18n/locales/pt-PT.json | 1 + internal/i18n/locales/zh.json | 1 + .../ai/azureaigateway/azureaigateway.go | 8 +++-- .../ai/azureaigateway/azureaigateway_test.go | 36 ++++++++++++++----- 13 files changed, 49 insertions(+), 15 deletions(-) diff --git a/docs/Azure-AI-Gateway.md b/docs/Azure-AI-Gateway.md index 416f511e..af76309a 100644 --- a/docs/Azure-AI-Gateway.md +++ b/docs/Azure-AI-Gateway.md @@ -173,7 +173,7 @@ fabric --youtube "https://youtube.com/watch?v=example" --model us.anthropic.clau curl -s https://example.com/article | fabric --model gpt-4o --pattern summarize # Create content from a prompt (Vertex AI) -fabric --model gemini-1.5-pro --pattern write_essay --stream +fabric --model gemini-1.5-pro --pattern write_essay ``` ### Switching Between Backends @@ -327,10 +327,10 @@ Fabric configuration is stored in `~/.config/fabric/.env`. Manual editing is sup ```bash # Example configuration -AZURE_AI_GATEWAY_BACKEND=bedrock -AZURE_AI_GATEWAY_GATEWAY_URL=https://gateway.company.com -AZURE_AI_GATEWAY_SUBSCRIPTION_KEY=your-key-here -AZURE_AI_GATEWAY_API_VERSION=2025-04-01-preview +AZUREAIGATEWAY_BACKEND=bedrock +AZUREAIGATEWAY_GATEWAY_URL=https://gateway.company.com +AZUREAIGATEWAY_SUBSCRIPTION_KEY=your-key-here +AZUREAIGATEWAY_API_VERSION=2025-04-01-preview ``` ## Support diff --git a/internal/i18n/locales/de.json b/internal/i18n/locales/de.json index cfb86311..706ed7c1 100644 --- a/internal/i18n/locales/de.json +++ b/internal/i18n/locales/de.json @@ -43,6 +43,7 @@ "azureaigateway_invalid_gateway_url": "ungültige Gateway-URL: %w", "azureaigateway_no_valid_messages": "keine gültigen Nachrichten nach Filterung leerer Inhalte", "azureaigateway_prepare_request_failed": "AzureAIGateway: %w", + "azureaigateway_response_too_large": "AzureAIGateway: Antwort zu groß (>%d Bytes)", "azureaigateway_subscription_key_question": "Geben Sie Ihren Azure APIM-Abonnementschlüssel ein", "azureaigateway_subscription_key_required": "Azure APIM-Abonnementschlüssel ist erforderlich", "azureaigateway_unsupported_backend": "nicht unterstütztes Backend: %s (gültige Optionen: bedrock, azure-openai, vertex-ai)", diff --git a/internal/i18n/locales/en.json b/internal/i18n/locales/en.json index fab24f6a..740aaf7a 100644 --- a/internal/i18n/locales/en.json +++ b/internal/i18n/locales/en.json @@ -43,6 +43,7 @@ "azureaigateway_invalid_gateway_url": "invalid gateway URL: %w", "azureaigateway_no_valid_messages": "no valid messages after filtering empty content", "azureaigateway_prepare_request_failed": "AzureAIGateway: %w", + "azureaigateway_response_too_large": "AzureAIGateway: response too large (>%d bytes)", "azureaigateway_subscription_key_question": "Enter your Azure APIM subscription key", "azureaigateway_subscription_key_required": "azure APIM subscription key is required", "azureaigateway_unsupported_backend": "unsupported backend: %s (valid options: bedrock, azure-openai, vertex-ai)", diff --git a/internal/i18n/locales/es.json b/internal/i18n/locales/es.json index 8e201c9b..7933b936 100644 --- a/internal/i18n/locales/es.json +++ b/internal/i18n/locales/es.json @@ -43,6 +43,7 @@ "azureaigateway_invalid_gateway_url": "URL de gateway inválida: %w", "azureaigateway_no_valid_messages": "sin mensajes válidos después de filtrar contenido vacío", "azureaigateway_prepare_request_failed": "AzureAIGateway: %w", + "azureaigateway_response_too_large": "AzureAIGateway: respuesta demasiado grande (>%d bytes)", "azureaigateway_subscription_key_question": "Ingrese su clave de suscripción de Azure APIM", "azureaigateway_subscription_key_required": "se requiere la clave de suscripción de Azure APIM", "azureaigateway_unsupported_backend": "backend no soportado: %s (opciones válidas: bedrock, azure-openai, vertex-ai)", diff --git a/internal/i18n/locales/fa.json b/internal/i18n/locales/fa.json index ccc48f62..13487c3b 100644 --- a/internal/i18n/locales/fa.json +++ b/internal/i18n/locales/fa.json @@ -43,6 +43,7 @@ "azureaigateway_invalid_gateway_url": "آدرس Gateway نامعتبر: %w", "azureaigateway_no_valid_messages": "هیچ پیام معتبری پس از فیلتر کردن محتوای خالی وجود ندارد", "azureaigateway_prepare_request_failed": "AzureAIGateway: %w", + "azureaigateway_response_too_large": "AzureAIGateway: پاسخ خیلی بزرگ است (>%d بایت)", "azureaigateway_subscription_key_question": "کلید اشتراک Azure APIM خود را وارد کنید", "azureaigateway_subscription_key_required": "کلید اشتراک Azure APIM الزامی است", "azureaigateway_unsupported_backend": "بک‌اند پشتیبانی نشده: %s (گزینه‌های معتبر: bedrock، azure-openai، vertex-ai)", diff --git a/internal/i18n/locales/fr.json b/internal/i18n/locales/fr.json index a13837ef..d990c812 100644 --- a/internal/i18n/locales/fr.json +++ b/internal/i18n/locales/fr.json @@ -43,6 +43,7 @@ "azureaigateway_invalid_gateway_url": "URL du gateway invalide : %w", "azureaigateway_no_valid_messages": "aucun message valide après filtrage du contenu vide", "azureaigateway_prepare_request_failed": "AzureAIGateway : %w", + "azureaigateway_response_too_large": "AzureAIGateway : réponse trop volumineuse (>%d octets)", "azureaigateway_subscription_key_question": "Entrez votre clé d'abonnement Azure APIM", "azureaigateway_subscription_key_required": "la clé d'abonnement Azure APIM est requise", "azureaigateway_unsupported_backend": "backend non pris en charge : %s (options valides : bedrock, azure-openai, vertex-ai)", diff --git a/internal/i18n/locales/it.json b/internal/i18n/locales/it.json index 9d854b6e..5b65be9f 100644 --- a/internal/i18n/locales/it.json +++ b/internal/i18n/locales/it.json @@ -43,6 +43,7 @@ "azureaigateway_invalid_gateway_url": "URL del gateway non valido: %w", "azureaigateway_no_valid_messages": "nessun messaggio valido dopo il filtraggio del contenuto vuoto", "azureaigateway_prepare_request_failed": "AzureAIGateway: %w", + "azureaigateway_response_too_large": "AzureAIGateway: risposta troppo grande (>%d byte)", "azureaigateway_subscription_key_question": "Inserire la propria chiave di sottoscrizione Azure APIM", "azureaigateway_subscription_key_required": "la chiave di sottoscrizione Azure APIM è obbligatoria", "azureaigateway_unsupported_backend": "backend non supportato: %s (opzioni valide: bedrock, azure-openai, vertex-ai)", diff --git a/internal/i18n/locales/ja.json b/internal/i18n/locales/ja.json index e2eaeee8..cb6b63e8 100644 --- a/internal/i18n/locales/ja.json +++ b/internal/i18n/locales/ja.json @@ -43,6 +43,7 @@ "azureaigateway_invalid_gateway_url": "無効なゲートウェイURL: %w", "azureaigateway_no_valid_messages": "空のコンテンツをフィルタリングした後、有効なメッセージがありません", "azureaigateway_prepare_request_failed": "AzureAIGateway: %w", + "azureaigateway_response_too_large": "AzureAIGateway: レスポンスが大きすぎます (>%dバイト)", "azureaigateway_subscription_key_question": "Azure APIMサブスクリプションキーを入力してください", "azureaigateway_subscription_key_required": "Azure APIMサブスクリプションキーは必須です", "azureaigateway_unsupported_backend": "サポートされていないバックエンド: %s（有効なオプション: bedrock、azure-openai、vertex-ai）", diff --git a/internal/i18n/locales/pt-BR.json b/internal/i18n/locales/pt-BR.json index 5327b204..8e410b41 100644 --- a/internal/i18n/locales/pt-BR.json +++ b/internal/i18n/locales/pt-BR.json @@ -43,6 +43,7 @@ "azureaigateway_invalid_gateway_url": "URL do gateway inválida: %w", "azureaigateway_no_valid_messages": "sem mensagens válidas após filtrar conteúdo vazio", "azureaigateway_prepare_request_failed": "AzureAIGateway: %w", + "azureaigateway_response_too_large": "AzureAIGateway: resposta muito grande (>%d bytes)", "azureaigateway_subscription_key_question": "Insira sua chave de assinatura do Azure APIM", "azureaigateway_subscription_key_required": "a chave de assinatura do Azure APIM é obrigatória", "azureaigateway_unsupported_backend": "backend não suportado: %s (opções válidas: bedrock, azure-openai, vertex-ai)", diff --git a/internal/i18n/locales/pt-PT.json b/internal/i18n/locales/pt-PT.json index 109329d5..2845b86c 100644 --- a/internal/i18n/locales/pt-PT.json +++ b/internal/i18n/locales/pt-PT.json @@ -43,6 +43,7 @@ "azureaigateway_invalid_gateway_url": "URL do gateway inválido: %w", "azureaigateway_no_valid_messages": "sem mensagens válidas após filtragem de conteúdo vazio", "azureaigateway_prepare_request_failed": "AzureAIGateway: %w", + "azureaigateway_response_too_large": "AzureAIGateway: resposta demasiado grande (>%d bytes)", "azureaigateway_subscription_key_question": "Introduza a sua chave de subscrição do Azure APIM", "azureaigateway_subscription_key_required": "a chave de subscrição do Azure APIM é obrigatória", "azureaigateway_unsupported_backend": "backend não suportado: %s (opções válidas: bedrock, azure-openai, vertex-ai)", diff --git a/internal/i18n/locales/zh.json b/internal/i18n/locales/zh.json index 069a8520..527235aa 100644 --- a/internal/i18n/locales/zh.json +++ b/internal/i18n/locales/zh.json @@ -43,6 +43,7 @@ "azureaigateway_invalid_gateway_url": "无效的网关 URL: %w", "azureaigateway_no_valid_messages": "过滤空内容后没有有效消息", "azureaigateway_prepare_request_failed": "AzureAIGateway: %w", + "azureaigateway_response_too_large": "AzureAIGateway: 响应太大 (>%d字节)", "azureaigateway_subscription_key_question": "输入您的 Azure APIM 订阅密钥", "azureaigateway_subscription_key_required": "Azure APIM 订阅密钥是必需的", "azureaigateway_unsupported_backend": "不支持的后端: %s（有效选项: bedrock、azure-openai、vertex-ai）", diff --git a/internal/plugins/ai/azureaigateway/azureaigateway.go b/internal/plugins/ai/azureaigateway/azureaigateway.go index 48c83e4e..cf2d1281 100644 --- a/internal/plugins/ai/azureaigateway/azureaigateway.go +++ b/internal/plugins/ai/azureaigateway/azureaigateway.go @@ -169,12 +169,16 @@ func (c *Client) Send(ctx context.Context, msgs []*chat.ChatCompletionMessage, o } defer resp.Body.Close() - // Limit response body size to 10MB to prevent memory exhaustion - limitedBody := io.LimitReader(resp.Body, 10*1024*1024) + // Read up to 10MB+1 byte to detect truncation + const maxResponseSize = 10 * 1024 * 1024 + limitedBody := io.LimitReader(resp.Body, maxResponseSize+1) respBody, err := io.ReadAll(limitedBody) if err != nil { return "", fmt.Errorf(i18n.T("azureaigateway_failed_read_response"), err) } + if len(respBody) > maxResponseSize { + return "", fmt.Errorf(i18n.T("azureaigateway_response_too_large"), maxResponseSize) + } debuglog.Debug(debuglog.Detailed, "AzureAIGateway response status: %d\n", resp.StatusCode) diff --git a/internal/plugins/ai/azureaigateway/azureaigateway_test.go b/internal/plugins/ai/azureaigateway/azureaigateway_test.go index e58e1bae..ea85271e 100644 --- a/internal/plugins/ai/azureaigateway/azureaigateway_test.go +++ b/internal/plugins/ai/azureaigateway/azureaigateway_test.go @@ -3,16 +3,29 @@ package azureaigateway import ( "context" "encoding/json" + "fmt" "io" "net/http" "net/http/httptest" + "os" "strings" "testing" "github.com/danielmiessler/fabric/internal/chat" "github.com/danielmiessler/fabric/internal/domain" + "github.com/danielmiessler/fabric/internal/i18n" ) +// TestMain pins the locale to English so that i18n.T() assertions +// are deterministic regardless of the CI machine's system locale. +func TestMain(m *testing.M) { + if _, err := i18n.Init("en"); err != nil { + fmt.Fprintf(os.Stderr, "failed to init i18n: %v\n", err) + os.Exit(1) + } + os.Exit(m.Run()) +} + // --- Bedrock Backend Tests --- func TestBedrockBuildEndpoint(t *testing.T) { @@ -732,13 +745,20 @@ func TestSendModelNotFoundError(t *testing.T) { } } +// failingRoundTripper always returns an error, simulating a network failure. +type failingRoundTripper struct{} + +func (f *failingRoundTripper) RoundTrip(*http.Request) (*http.Response, error) { + return nil, fmt.Errorf("connection refused") +} + func TestSendNetworkError(t *testing.T) { - // ISC-C17: Test unreachable gateway URL → connection error + // ISC-C17: Test network failure → connection error (deterministic, no real DNS) c := NewClient() - c.GatewayURL.Value = "https://non-existent-gateway-12345.invalid" + c.GatewayURL.Value = "https://gateway.example.com" c.SubscriptionKey.Value = "test-key" c.BackendType.Value = "bedrock" - c.httpClient = &http.Client{Timeout: gatewayTimeout} + c.httpClient = &http.Client{Transport: &failingRoundTripper{}} c.backend = NewBedrockBackend("test-key") msgs := []*chat.ChatCompletionMessage{ @@ -752,7 +772,7 @@ func TestSendNetworkError(t *testing.T) { _, err := c.Send(context.Background(), msgs, opts) if err == nil { - t.Fatal("Send() expected error for unreachable gateway") + t.Fatal("Send() expected error for network failure") } if !strings.Contains(err.Error(), "HTTP request failed") { t.Errorf("error should mention HTTP request failure: %v", err) @@ -903,11 +923,11 @@ func TestSendStreamFallback(t *testing.T) { // --- ISC-C18: API Version Compatibility Test --- func TestAzureOpenAIAPIVersionCompatibility(t *testing.T) { - // ISC-C18: Azure OpenAI API version 2025-04-01-preview compatibility with Azure APIM Gateway + // ISC-C18: Azure OpenAI API version compatibility with Azure APIM Gateway // Reference: https://learn.microsoft.com/en-us/azure/ai-services/openai/api-version-deprecation - // This test verifies that the API version in the endpoint is compatible with Azure APIM Gateway. - // The version 2024-10-21 is currently used, which is compatible with APIM gateways. - // When updating to 2025-04-01-preview, ensure APIM gateway supports the new version. + // This test verifies that the default API version in the endpoint (currently 2025-04-01-preview) + // is explicitly set and is compatible with Azure APIM Gateway. When changing the default API + // version in the backend, ensure that APIM gateways are updated to support the new version. b := NewAzureOpenAIBackend("key", "") endpoint := b.BuildEndpoint("https://gw.example.com", "gpt-4")