Skip to content

Commit 0d6fa6c

Browse files
fix: validate Postgres connection string, bind EmbeddingRetry from config, remove resilience handler from local AI client
- IsValidNpgsqlConnectionString() helper validates that a connection string has a non-empty Host instead of just checking for non-whitespace text, preventing the 'your-postgres-connection-string-here' placeholder from passing validation. - AddOptions<EmbeddingRetryOptions>().Bind() wired in AddConfiguredChatServices so retry config from appsettings is not silently ignored when using the AIOptions overload of AddAzureOpenAIServices. - .RemoveAllResilienceHandlers() added to LocalAIChat HttpClient to prevent the global ConfigureHttpClientDefaults resilience handler (30s attempt / 90s total timeouts) from cutting off long LLM completions. EXTEXP0001 suppressed in the project file.
1 parent 413e352 commit 0d6fa6c

2 files changed

Lines changed: 37 additions & 2 deletions

File tree

EssentialCSharp.Chat.Shared/EssentialCSharp.Chat.Common.csproj

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,17 @@
22

33
<PropertyGroup>
44
<TargetFramework>net10.0</TargetFramework>
5+
<!-- Suppress experimental API warning from RemoveAllResilienceHandlers() used to opt out
6+
the local AI HttpClient from the global standard resilience handler. -->
7+
<NoWarn>$(NoWarn);EXTEXP0001</NoWarn>
58
</PropertyGroup>
69

710
<ItemGroup>
811
<PackageReference Include="Azure.Identity" />
912
<PackageReference Include="Microsoft.Extensions.AI.OpenAI" />
1013
<PackageReference Include="Microsoft.SemanticKernel" />
1114
<PackageReference Include="Microsoft.SemanticKernel.Connectors.PgVector" />
15+
<PackageReference Include="Microsoft.Extensions.Http.Resilience" />
1216
<PackageReference Include="ModelContextProtocol" />
1317
<PackageReference Include="ModelContextProtocol.AspNetCore" />
1418
<PackageReference Include="Microsoft.SourceLink.GitHub">

EssentialCSharp.Chat.Shared/Extensions/ServiceCollectionExtensions.cs

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using Microsoft.Extensions.AI;
77
using Microsoft.Extensions.Configuration;
88
using Microsoft.Extensions.DependencyInjection;
9+
using Microsoft.Extensions.Http.Resilience;
910
using Microsoft.Extensions.Options;
1011
using Microsoft.SemanticKernel;
1112
using Npgsql;
@@ -101,7 +102,8 @@ public static IServiceCollection AddConfiguredChatServices(this IServiceCollecti
101102
bool hasAzureEndpoint = !string.IsNullOrWhiteSpace(aiOptions.Endpoint);
102103
bool hasAzureChatDeployment = !string.IsNullOrWhiteSpace(aiOptions.ChatDeploymentName);
103104
bool hasAzureVectorDeployment = !string.IsNullOrWhiteSpace(aiOptions.VectorGenerationDeploymentName);
104-
bool hasAzureConfig = hasAzureEndpoint && hasAzureChatDeployment && hasAzureVectorDeployment && !string.IsNullOrWhiteSpace(postgresConnectionString);
105+
bool hasAzureConfig = hasAzureEndpoint && hasAzureChatDeployment && hasAzureVectorDeployment
106+
&& IsValidNpgsqlConnectionString(postgresConnectionString);
105107

106108
string localEndpoint = ResolveLocalEndpoint(aiOptions, configuration);
107109
bool hasLocalConfig = aiOptions.UseLocalAI
@@ -120,6 +122,10 @@ public static IServiceCollection AddConfiguredChatServices(this IServiceCollecti
120122
else
121123
{
122124
services.AddAzureOpenAIServices(aiOptions, postgresConnectionString!);
125+
// Bind EmbeddingRetry from config so operator appsettings/env overrides are honored.
126+
// The AIOptions overload of AddAzureOpenAIServices only registers validation, not config binding.
127+
services.AddOptions<EmbeddingRetryOptions>()
128+
.Bind(configuration.GetSection(EmbeddingRetryOptions.SectionPath));
123129
services.AddSingleton<IChatCompletionService>(provider => provider.GetRequiredService<AIChatService>());
124130
Console.WriteLine("[AI] Selected backend: Azure/Foundry.");
125131
return services;
@@ -140,7 +146,12 @@ public static IServiceCollection AddConfiguredChatServices(this IServiceCollecti
140146
{
141147
client.BaseAddress = localEndpointUri;
142148
client.Timeout = TimeSpan.FromSeconds(120);
143-
});
149+
})
150+
// Disable the global standard resilience handler (set by ConfigureHttpClientDefaults
151+
// in Program.cs). Its default attempt timeout (30s) and total timeout (90s) would
152+
// cut off long local-LLM completions. We set HttpClient.Timeout directly instead.
153+
// Retries are also wrong for LLM calls (non-idempotent, partial responses).
154+
.RemoveAllResilienceHandlers();
144155
services.AddSingleton<IChatCompletionService, LocalChatService>();
145156
Console.WriteLine("[AI] Selected backend: Local (Ollama/OpenAI-compatible).");
146157
return services;
@@ -274,4 +285,24 @@ private static string ResolveLocalEndpoint(AIOptions options, IConfiguration con
274285
return configuration.GetConnectionString("ollama-chat") ?? string.Empty;
275286
}
276287

288+
/// <summary>
289+
/// Returns true if <paramref name="connectionString"/> can be parsed by
290+
/// <see cref="NpgsqlConnectionStringBuilder"/> and resolves to a non-empty Host.
291+
/// Rejects null, empty, and placeholder strings like "your-postgres-connection-string-here".
292+
/// </summary>
293+
private static bool IsValidNpgsqlConnectionString(string? connectionString)
294+
{
295+
if (string.IsNullOrWhiteSpace(connectionString))
296+
return false;
297+
try
298+
{
299+
var builder = new NpgsqlConnectionStringBuilder(connectionString);
300+
return !string.IsNullOrWhiteSpace(builder.Host);
301+
}
302+
catch (Exception)
303+
{
304+
return false;
305+
}
306+
}
307+
277308
}

0 commit comments

Comments
 (0)