ChatClient demo with API for secure secret retrieval #659
Conversation
|
There was a problem hiding this comment.
Pull Request Overview
This PR implements a comprehensive ChatClient demo that showcases a secure client-side AI architecture using .NET MAUI and Microsoft.Extensions.AI. The solution includes a minimal Web API for credential distribution and a full-featured MAUI client with AI function calling capabilities and secure credential storage.
Key changes:
- Complete .NET MAUI client application with secure credential management using SecureStorage
- Minimal ASP.NET Core Web API for one-time credential distribution
- Full suite of AI tools (Weather, Calculator, File Operations, System Info, Timer) implemented as AIFunction subclasses
Reviewed Changes
Copilot reviewed 62 out of 71 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| ChatMobile.Api/Program.cs | Minimal Web API with CORS and credential distribution endpoint |
| ChatMobile.Client/Services/SecureCredentialService.cs | Implements secure storage of AI credentials using MAUI SecureStorage |
| ChatMobile.Client/Services/ChatService.cs | Core AI chat service with Microsoft.Extensions.AI client and function calling |
| ChatMobile.Client/Tools/ | Complete set of AI function tools for weather, calculator, file operations, system info, and timers |
| ChatMobile.Client/ViewModels/ | MVVM implementation for chat and setup functionality |
| ChatMobile.Client/Views/ | XAML UI with rich tool result display and credential setup interface |
| FontSize="12" | ||
| TextColor="#666" | ||
| VerticalOptions="Center" | ||
| IsVisible="{Binding IsDirectory, Converter={x:Null}}"/> |
There was a problem hiding this comment.
The converter binding is set to '{x:Null}' which will cause a binding error. This should be '{Binding IsDirectory, Converter={StaticResource InvertedBoolConverter}}' to show file size only for files, not directories.
| IsVisible="{Binding IsDirectory, Converter={x:Null}}"/> | |
| IsVisible="{Binding IsDirectory, Converter={StaticResource InvertedBoolConverter}}"/> |
Copilot uses AI. Check for mistakes.
|
|
||
| var endpoint = await _credentialService.GetAzureOpenAIEndpointAsync(); | ||
| var apiKey = await _credentialService.GetAzureOpenAIApiKeyAsync(); | ||
| var model = await _credentialService.GetAzureOpenAIModelAsync() ?? "gpt-5-mini"; |
There was a problem hiding this comment.
The default model 'gpt-5-mini' does not exist. It should be 'gpt-4o-mini' to match the pattern used elsewhere in the codebase.
| var model = await _credentialService.GetAzureOpenAIModelAsync() ?? "gpt-5-mini"; | |
| var model = await _credentialService.GetAzureOpenAIModelAsync() ?? "gpt-4o-mini"; |
Copilot uses AI. Check for mistakes.
| builder.Services.AddHttpClient<IApiService, ApiService>(client => | ||
| { | ||
| // Use IP address instead of localhost to avoid DNS issues | ||
| client.BaseAddress = new Uri("http://127.0.0.1:5132/"); |
There was a problem hiding this comment.
Hardcoded localhost URL should be moved to configuration. Consider using the appsettings.json BaseUrl or environment-specific configuration to avoid hardcoded network addresses in production code.
| client.BaseAddress = new Uri("http://127.0.0.1:5132/"); | |
| var configuration = builder.Configuration; | |
| var apiBaseUrl = configuration["ApiBaseUrl"] ?? "http://127.0.0.1:5132/"; | |
| builder.Services.AddHttpClient<IApiService, ApiService>(client => | |
| { | |
| // Use configured API base URL, fallback to localhost if not set | |
| client.BaseAddress = new Uri(apiBaseUrl); |
Copilot uses AI. Check for mistakes.
| try | ||
| { | ||
| var result = await SecureStorage.GetAsync(AZURE_ENDPOINT_KEY); | ||
| System.Diagnostics.Debug.WriteLine($"SecureCredentialService: GetAzureOpenAIEndpointAsync returned: {result}"); |
There was a problem hiding this comment.
Debug logging of credentials poses a security risk. Even though this logs the endpoint (not API key), credential-related information should not be logged in production builds. Consider removing or wrapping in conditional compilation directives.
| System.Diagnostics.Debug.WriteLine($"SecureCredentialService: GetAzureOpenAIEndpointAsync returned: {result}"); | |
| var result = await SecureStorage.GetAsync(AZURE_ENDPOINT_KEY); | |
| #if DEBUG | |
| System.Diagnostics.Debug.WriteLine($"SecureCredentialService: GetAzureOpenAIEndpointAsync returned: {result}"); | |
| #endif |
Copilot uses AI. Check for mistakes.
| timer.Dispose(); | ||
| _activeTimers.Remove(timerId); | ||
| } | ||
| Console.WriteLine($"🔔 Timer '{title}' finished ({timerId})."); |
There was a problem hiding this comment.
Using Console.WriteLine in a MAUI application is not appropriate. This should use proper logging through ILogger or raise an event/notification that the UI can handle appropriately.
| Console.WriteLine($"🔔 Timer '{title}' finished ({timerId})."); | |
| Logger?.LogInformation("🔔 Timer '{Title}' finished ({TimerId}).", title, timerId); |
Copilot uses AI. Check for mistakes.
| <!-- Chat messages --> | ||
| <CollectionView Grid.Row="0" | ||
| x:Name="MessagesView" | ||
| ItemsSource="{Binding Messages}" |
There was a problem hiding this comment.
[nitpick] CollectionView with complex DataTemplates containing multiple conditional visibility bindings and nested CollectionViews could impact scrolling performance. Consider using data template selectors or virtualization optimizations for better performance with large message lists.
| ItemsSource="{Binding Messages}" | |
| ItemsSource="{Binding Messages}" | |
| ItemTemplate="{StaticResource MessageTemplateSelector}" |
Copilot uses AI. Check for mistakes.
|
|
||
| <TabBar> | ||
| <ShellContent | ||
| Title="Chat" |
There was a problem hiding this comment.
The sample use a nice UI, could we include small details like icons here?
| - "List the files in my Documents folder" | ||
| - "Show the top 10 largest files in Downloads" | ||
|
|
||
| ### System Info (Mobile) |
There was a problem hiding this comment.
Could include a note indication that some tools are fake ones? (not important for the sample), like getting the battery.
Example:
Some tools used in this sample, such as battery level retrieval, are mock implementations and do not reflect actual device APIs. They are included for illustrative purposes only and may not function in a production environment.
3 participants
API holds the secrets
App "authenticates" and retrieves secrets, storing them in
SecureStorageApp uses IChatClient and local tools