fix: AI, lifecycle improvements, etc#990
Merged
BenjaminMichaelis merged 9 commits intomainfrom Apr 17, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the web app and chat services to address AI chat reliability issues (notably managed identity / token expiry behaviors), while also modernizing lifecycle/ops concerns like health endpoints, OpenTelemetry wiring, and dependency updates for .NET 10.
Changes:
- Reworks OpenTelemetry + health check setup in
Program.cs(including new/healthand/aliveendpoints) and adds HttpClient resilience defaults. - Improves PostgreSQL managed identity token handling for the vector store and adds a targeted retry for token-expiry failures during vector search.
- Updates project dependencies and configuration/docs for .NET 10 and related package/security pinning.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/getting-started.md | Updates local dev prerequisites to .NET 10 SDK. |
| EssentialCSharp.Web/Services/ListingSourceCodeService.cs | Allows overriding listing source root via LISTING_SOURCE_CODE_PATH. |
| EssentialCSharp.Web/Program.cs | Adds health checks, revamps OpenTelemetry export path selection, adds HttpClient resilience defaults, and changes health endpoints. |
| EssentialCSharp.Web/EssentialCSharp.Web.csproj | Moves to net10.0 and adds OpenTelemetry + resilience packages; removes profiler reference. |
| EssentialCSharp.Web/DatabaseMigrationService.cs | Simplifies startup migration logic to always run EF migrations. |
| EssentialCSharp.Web.Tests/WebApplicationFactory.cs | Removes DatabaseMigrationService in tests to avoid MigrateAsync vs EnsureCreated conflicts. |
| EssentialCSharp.Chat.Shared/Services/AISearchService.cs | Adds cancellation support and retries once on specific auth failures; returns materialized results. |
| EssentialCSharp.Chat.Shared/Services/AIChatService.cs | Adapts to new vector search return type and propagates cancellation. |
| EssentialCSharp.Chat.Shared/Extensions/ServiceCollectionExtensions.cs | Switches PostgreSQL MI auth to per-connection token refresh via UsePasswordProvider and sets connection lifetime. |
| Directory.Packages.props | Updates/introduces package versions (OpenTelemetry, resilience, NuGet security pinning, SourceLink). |
…eProvider disposal, retry tests
- Fix CI failure: update FunctionalTests.cs to use /health and /alive instead of removed /healthz
- Fix AppInsights silent failure: check APPLICATIONINSIGHTS_CONNECTION_STRING,
ApplicationInsights:ConnectionString, and GetConnectionString('ApplicationInsights') fallback;
pass resolved string explicitly to UseAzureMonitor(options => ...) since the no-arg overload
only auto-reads the flat env var. Deployment workflow sets ApplicationInsights__ConnectionString
which maps to the hierarchical key, not the flat env var.
- Fix PhysicalFileProvider IDisposable leak: ListingSourceCodeService implements IDisposable with
_OwnsFileProvider tracking; only disposes the provider it creates, not the framework-managed
ContentRootFileProvider; GC.SuppressFinalize included per CA1816.
- Add AISearchService retry unit tests: 4 tests covering happy path (no retry), 28000 retry
success, non-28000 pass-through, and both-attempts-fail propagation. PostgresException has a
public constructor so no reflection or seams are needed.
- Extract TestListingSourceCodeServiceHelper with ResolveTestDataPath() and CreateService() so both ListingSourceCodeServiceTests and WebApplicationFactory share the same TestData-backed ListingSourceCodeService setup. Also aligns failure behavior: both now throw a clear InvalidOperationException if the TestData directory is missing. - Add SetupSearch() helper in AISearchServiceTests that returns the ISetup for SearchAsync, removing the repeated 5-line Moq matcher block from all 4 test bodies. Each test still specifies its own .Returns(...) inline so retry behavior stays visible.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| AutoMocker mocker = new(); | ||
| mocker.Setup<IWebHostEnvironment, string>(m => m.ContentRootPath).Returns(testDataPath); | ||
| mocker.Setup<IWebHostEnvironment, IFileProvider>(m => m.ContentRootFileProvider) | ||
| .Returns(new PhysicalFileProvider(testDataPath)); |
Root cause: workflow was setting ApplicationInsights__ConnectionString (double-underscore hierarchical .NET config key) which does NOT match the flat env var that UseAzureMonitor() and Aspire's AzureApplicationInsightsResource both expect. - workflow: rename ApplicationInsights__ConnectionString -> APPLICATIONINSIGHTS_CONNECTION_STRING (both Staging and Production deploy steps; secret reference unchanged) - Program.cs: remove 3-key fallback chain; single builder.Configuration["APPLICATIONINSIGHTS_CONNECTION_STRING"] check; simplify UseAzureMonitor() to no-arg form (auto-reads the canonical env var) Official docs: 'The Azure Monitor distro expects the environment variable to be APPLICATIONINSIGHTS_CONNECTION_STRING' (learn.microsoft.com/dotnet/aspire)
BenjaminMichaelis
added a commit
that referenced
this pull request
Apr 25, 2026
## Description Fix the AI issues. Need to consider a different store as semantic kernel and postgres are a bit behind.... Fixes #886 ### Ensure that your pull request has followed all the steps below: - [ ] Code compilation - [ ] Created tests which fail without the change (if possible) - [ ] All tests passing - [ ] Extended the README / documentation, if necessary
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fix the AI issues. Need to consider a different store as semantic kernel and postgres are a bit behind....
Fixes #886
Ensure that your pull request has followed all the steps below: