Phase 1: Add AppChat sessions powered by on-device SLM

[mattleibow]

Implement local AI-powered chat sessions using Microsoft.Extensions.AI and Microsoft Agent Framework. AppChat sessions appear alongside Copilot sessions in the sidebar with distinct visual treatment.

New files:

• Models/SessionKind.cs: Copilot | AppChat enum
• Services/AI/DirectLocalChatService.cs: Wraps ChatClientAgent with AgentSession per chat, streaming via RunStreamingAsync
• Services/AI/AppChatTools.cs: Read-only tools (GetSessionList, GetOrganizationState, GetQueuedMessages, GetRecentErrors)
• Services/AI/NonFunctionInvokingChatClient.cs: Workaround for FunctionInvokingChatClient should not re-invoke already-handled FunctionCallContent dotnet/extensions#7204 (double tool invocation)

Key changes:

• SessionState.Session is now nullable (CopilotSession?)
• AgentSessionInfo gains Kind property (defaults to Copilot)
• CopilotService routes SendPromptAsync by SessionKind
• MauiProgram registers keyed 'local' IChatClient pipeline
• ExpandedSessionView hides Copilot-only controls for AppChat (Fiesta, ReflectionCycle, Plan/Autopilot, model selector)
• Sidebar gets '✨ App Chat' button in both desktop and flyout

Found a bug: dotnet/maui#34124

[PureWeen]

🔍 Multi-Model Code Review — PR #204

PR: Phase 1: Add AppChat sessions powered by on-device SLM
Reviewers: Claude Opus 4.6 · Claude Sonnet 4.6 · GPT-5.3-Codex
CI Status: ⚠️ No CI checks configured on this branch
Prior Reviews: None
Files: 11 changed (+611/−6)

---

Consensus Findings (2+ of 3 models)

🔴 CRITICAL — DI crash on Android/Windows

File: AppChatPopover.razor L4, MauiProgram.cs RegisterLocalAI
Flagged by: Opus 🟥 · Sonnet 🟥 · Codex 🟥 (3/3)

@inject DirectLocalChatService LocalChat is unconditional in AppChatPopover.razor, which is rendered from MainLayout.razor on all platforms. But DirectLocalChatService is only registered inside #if MACCATALYST || IOS. On Android/Windows, the DI container throws InvalidOperationException at component creation — crashing the entire layout.

Fix: Either (a) guard <AppChatPopover> with a platform check, (b) register a no-op stub on non-Apple platforms, or (c) conditionally compile the component.

---

🔴 CRITICAL — Missing @implements IDisposable — Dispose() never called

File: AppChatPopover.razor — Dispose() method
Flagged by: Opus 🟥 · Sonnet 🟡 · Codex 🟡 (3/3)

The component defines public void Dispose() (cancels CTS, removes session) but has no @implements IDisposable directive. Blazor will never call Dispose() on teardown. Result: CancellationTokenSource leaks, and AgentSession entries accumulate forever in DirectLocalChatService._sessions.

Fix: Add @implements IDisposable at the top of the file.

---

🟡 MODERATE — SwitchToSession tool mutates UI state from background thread

File: AppChatTools.cs — SwitchToSession method (~lines 63–72)
Flagged by: Opus 🟡 · Sonnet 🟥 · Codex 🟡 (3/3)

SwitchToSession calls service.SwitchSession() (fires OnStateChanged) and service.SaveUiState() from the AI agent's tool execution context — a background thread. The codebase requires all CopilotService UI mutations to be marshaled through InvokeOnUI(). Also mutates session.LastReadMessageCount without synchronization while the UI thread may be reading it.

Fix: Tool should return data only; the component should apply state changes on the UI thread. Or post the switch via SynchronizationContext.

---

🟡 MODERATE — GetOrCreateAgent() race condition — double-create

File: DirectLocalChatService.cs — lines 41–48
Flagged by: Opus 🟡 · Sonnet 🟡 · Codex 🟥 (3/3)

if (_agent is not null) return _agent;
// ← two threads can both pass the null check
_agent = new ChatClientAgent(...);

No lock or volatile on _agent. Two concurrent calls create two agents, one silently discarded. Use Lazy<T> or Interlocked.CompareExchange.

---

🟡 MODERATE — GetOrCreateSessionAsync() TOCTOU race

File: DirectLocalChatService.cs — lines 50–58
Flagged by: Opus 🟡 · Sonnet 🟡 · Codex 🟥 (3/3)

if (_sessions.TryGetValue(sessionName, out var existing)) return existing;
var session = await agent.CreateSessionAsync();  // yields — widens race window
_sessions[sessionName] = session;

Two concurrent calls for the same key both miss TryGetValue, both create sessions, one is silently discarded (resource leak).

Fix: Use GetOrAdd with lazy factory, or serialize per-key creation.

---

🟡 MODERATE — Background thread mutations of component state

File: AppChatPopover.razor — all callbacks in SendMessage (~lines 80–143)
Flagged by: Opus 🟡 · Sonnet 🟥 · Codex (implicit in DI finding) (2/3)

The onComplete, onError, onToolStart, onToolEnd callbacks mutate messages (a non-thread-safe List<T>), isProcessing, streamingContent, etc. before calling InvokeAsync(StateHasChanged). If Blazor's render cycle enumerates messages concurrently, InvalidOperationException or silent corruption results.

Fix: Move all state mutations inside the InvokeAsync callback.

---

🟡 MODERATE — Dashboard guard removal changes behavior

File: Dashboard.razor — else-if condition (~line 958)
Flagged by: Opus 🟡 · Sonnet 🟡 · Codex 🟡 (3/3)

Removing expandedSession != null means when expandedSession is null and active is non-null, the branch now fires (null != active → true), auto-expanding sessions even when the user hadn't expanded anything. Also calls switchKeepAliveSlot for a potentially invalid slot.

---

🟢 MINOR — CancellationTokenSource not disposed before reassignment

File: AppChatPopover.razor — SendMessage (~lines 87–89)
Flagged by: Opus 🟢 · Sonnet 🟢 · Codex 🟢 (3/3)

_cts?.Cancel() is called but Dispose() is not, before creating a new CTS. Leaks timer handles.

Fix: _cts?.Cancel(); _cts?.Dispose(); _cts = new CancellationTokenSource();

---

🟢 MINOR — No tests for new code paths

File: PolyPilot.Tests/
Flagged by: Opus (implicit) · Sonnet (implicit) · Codex 🟢 (3/3)

Zero test coverage for DirectLocalChatService (concurrency, threading), AppChatTools (UI-thread safety), or AppChatPopover (lifecycle, DI gating). The test csproj adds <Compile Include> links for the new files but no actual test classes.

---

Summary

#	Severity	Finding	Models
1	🔴	DI crash on Android/Windows	3/3
2	🔴	Missing @implements IDisposable	3/3
3	🟡	SwitchToSession background thread mutation	3/3
4	🟡	GetOrCreateAgent race condition	3/3
5	🟡	GetOrCreateSessionAsync TOCTOU race	3/3
6	🟡	Callback state mutations off UI thread	2/3
7	🟡	Dashboard guard removal regression	3/3
8	🟢	CTS not disposed before reassignment	3/3
9	🟢	No test coverage for new code	3/3

Recommended Action: ⚠️ Request Changes

The two CRITICAL issues must be fixed before merge:

1. DI crash: DirectLocalChatService injection will crash the app on Android/Windows
2. Missing IDisposable: Resource and session leaks on every component teardown

The 5 MODERATE issues (thread safety, race conditions, dashboard regression) should also be addressed — they will cause intermittent bugs under real-world use.