Refactor: strip heavy-lifting from AgentSessionShortcutContext; centralize AgentServices/MCP/persistence composition

[JoshuaRowePhantom]

Summary

AgentSessionShortcutContext (features/Phantom.Workspaces/ViewModels/AgentSessionShortcutContext.cs) is a GUI ViewModels-layer class, but it performs a large amount of composition and infrastructure heavy-lifting that belongs in lower layers:

1. AgentServices composition — hand-assembles the toolset-factory chain, the MCP tool-resource factory (machine/user/defaults mcp-servers search prefixes), the account-upsert service, the current-session context, and forwards SecretProvider.
2. Agent-persistence-store construction — a ~95-line switch over RepositorySource that builds Web / DevTunnel (reconnecting) / MongoDB / in-memory stores, including GitHub-auth-token resolution and chat-history-provider wiring.
3. Agent-session entity authoring — builds the agent-session entity JSON document, names, display name, parameter-values, host-profile id, and name sanitization.

Because each launch path re-assembles AgentServices by hand, the assemblies drift and silently drop services. This is not hypothetical — it is the direct root cause of two filed bugs:

• Interactive MCP OAuth not wired on session-launch path: AgentServices omits McpOAuthOptions #1402 — this class's AgentServices omits McpOAuthOptions, so interactive MCP OAuth falls back to the failing "not configured" stub on session launch.
• Secret materialization is gated on the manifest; agent-session launches never resolve ${SECRET:...} (only sessions/definitions should materialize secrets) #1401 — the session-launch path drops manifest-driven SecretProvider materialization.

Both are symptoms of the same architectural problem: service composition is duplicated across App.axaml.cs and AgentSessionShortcutContext (and other launch sites) instead of living in one shared composition root. The goal of this bug is to strip AgentSessionShortcutContext down to a thin GUI adapter and move the heavy lifting (especially all MCP-server work) into the appropriate layers, so every launch path gets one complete, identical AgentServices bundle and the whole class of "forgot to thread service X" bugs is eliminated.

Root Cause

AgentSessionShortcutContext mixes four unrelated responsibilities in the GUI layer.

(a) AgentServices composition (37-86) — including the MCP tool-resource factory (101-129):

ToolResourceFactory = this.CreateToolResourceFactory(dataAccessLayer),   // 79
...
private IToolResourceFactory CreateToolResourceFactory(IDataAccessLayer dataAccessLayer)
{
    ...
    return new ComposingToolResourceFactory(
        new FixedToolResourceFactory(CreateFixedToolMapping()),
        new McpServerEntityToolResourceFactory(dataAccessLayer, [ machineProfilePrefix, ${USER}/mcp-servers, defaults/mcp-servers ]));  // 115-128
}

The returned AgentServices (74-85) sets SecretProvider but not McpOAuthOptions (#1402), and the app-level composition in App.axaml.cs:313-316 builds a different AgentServices that does have McpOAuthOptions. Two hand-written bundles, guaranteed to drift.

(b) Persistence-store construction (197-292) — a RepositorySource switch that belongs in a persistence-store factory:

private static async Task<IAgentPersistenceStore> CreateAgentPersistenceStoreAsync(RepositorySource repositorySource)
    => repositorySource switch
    {
        WebRepositorySource webSource => CreateWebAgentPersistenceStore(webSource),
        DevTunnelNameRepositorySource devTunnelSource => await CreateDevTunnelAgentPersistenceStoreAsync(devTunnelSource),
        MongoDbRepositorySource mongoSource => await CreateMongoDbAgentPersistenceStoreAsync(mongoSource),
        _ => AgentPersistenceStoreFactory.CreateInMemory(),
    };

AgentPersistenceStoreFactory already exists (used at 222/276/291) — this switch and its three helpers should live there (or in an IAgentPersistenceStoreFactory keyed on RepositorySource), not in a GUI ViewModel.

(c) Agent-session entity authoring (138-195, 294-369) — CreateAgentSessionEntityData, CreateSessionObjectSimpleName, SanitizeNameComponent are data-layer concerns.

(d) The only genuinely GUI-specific inputs are MainWindowViewModel, ShortcutManager, and WorkspaceGuiContextProvider. Everything else is host/infrastructure composition.

Affected Files

File	What it contributes
features/Phantom.Workspaces/ViewModels/AgentSessionShortcutContext.cs	The bloated class: AgentServices composition (37-86), MCP tool-resource factory (101-129), persistence-store switch (197-292), entity authoring (138-195, 294-369).
features/Phantom.Workspaces/App.axaml.cs	Builds a parallel AgentServices (313-316) with McpOAuthOptions; target caller of the shared composition root.
features/Phantom.Workspaces/Services/ApplicationServices.cs	Holds process-wide services (SecretProvider); natural home for (or reference to) the shared AgentServices composition + McpOAuthOptions.
features/Phantom.Workspaces.Llm.Interfaces/AgentServices.cs	The bundle being assembled; should be produced by one factory, not hand-written per site.
features/Phantom.Workspaces/Services/Mcp/McpOAuthComposition.cs	MCP OAuth options bundle that must be part of the shared composition.
features/Phantom.Workspaces.Llm.Core/* (AgentPersistenceStoreFactory, ToolResourceFactory, McpServerEntityToolResourceFactory)	Existing lower-layer factories the extracted logic should move into or be driven by.

Design / Fix

Reduce AgentSessionShortcutContext to a thin GUI adapter; move each responsibility to its proper layer. Prefer extracting into existing factories over inventing new abstractions.

1. Single AgentServices composition root. Introduce one host-level composer (e.g. Services/AgentServicesComposition.cs, or a method on ApplicationServices) that produces the complete AgentServices bundle: toolset-factory chain, MCP tool-resource factory, SecretProvider, McpOAuthOptions, account-upsert service, current-session context. Both App.axaml.cs and AgentSessionShortcutContext (and any other session-launch site) call this one method, passing only the small GUI-specific context (MainWindowViewModel/ShortcutManager/WorkspaceGuiContextProvider). This structurally prevents Secret materialization is gated on the manifest; agent-session launches never resolve ${SECRET:...} (only sessions/definitions should materialize secrets) #1401/Interactive MCP OAuth not wired on session-launch path: AgentServices omits McpOAuthOptions #1402-class drift — there is exactly one place that can forget a service, and it forgets it for everyone (caught by one test) rather than silently for one path.

2. Move MCP-server tool-resource composition out of the GUI. CreateToolResourceFactory / CreateFixedToolMapping (the ComposingToolResourceFactory + McpServerEntityToolResourceFactory + machine/user/defaults prefixes) belong in an Llm.Core factory (e.g. extend ToolResourceFactory), parameterized by the data-access layer and execution context. The GUI should not know MCP search-prefix precedence.

3. Move persistence-store construction into AgentPersistenceStoreFactory. Relocate CreateAgentPersistenceStoreAsync and its Web/DevTunnel/MongoDB helpers into AgentPersistenceStoreFactory (or a new IAgentPersistenceStoreFactory keyed on RepositorySource). AgentSessionShortcutContext should call one factory method, not own the switch.

4. Move agent-session entity authoring into the data layer. Extract CreateAgentSessionEntityData, CreateSessionObjectSimpleName, SanitizeNameComponent into a data-layer AgentSessionEntityFactory (or similar). The shortcut context orchestrates the UpdateAsync call; it does not build the document.

5. Result. AgentSessionShortcutContext shrinks to: resolve GUI context → call the shared AgentServices composition → call the persistence-store factory → call the entity factory → issue the entity UpdateAsync. No MCP wiring, no persistence switch, no JSON authoring.

This bug is the umbrella refactor; #1401 and #1402 are its symptoms. Landing this eliminates the root cause (per-site hand-assembly of AgentServices). #1401/#1402 may be fixed first as tactical patches, or folded into this refactor — either way their fixes should converge on the single composition root.

Considered / Background

• A lighter alternative is to only fix the immediate omissions (thread McpOAuthOptions, keep everything else in place). That is Interactive MCP OAuth not wired on session-launch path: AgentServices omits McpOAuthOptions #1402's scope and resolves the current failure, but leaves the duplication that will keep producing drift bugs. This bug exists specifically to remove that duplication.

Expected Tests

Test Name	Class	What It Verifies
AgentServicesComposition_Compose_ProducesCompleteBundle	AgentServicesCompositionTests	The single composition root returns AgentServices with SecretProvider, McpOAuthOptions, toolset factory, MCP tool-resource factory, account-upsert service, and current-session context all set.
AgentSessionShortcutContext_CreateAgentServices_DelegatesToCompositionRoot	AgentSessionShortcutContextTests	The session-launch path returns the bundle from the shared composition root (same McpOAuthOptions/SecretProvider instances as the app-level path), not a hand-assembled one.
AgentSessionShortcutContext_DoesNotConstructPersistenceStoreOrToolResourceFactory	AgentSessionShortcutContextTests	The class no longer owns the RepositorySource switch or MCP tool-resource composition (delegates to the factories).
AgentPersistenceStoreFactory_CreateForRepositorySource_Web	AgentPersistenceStoreFactoryTests	The Web branch is created by the factory (moved out of the GUI).
AgentPersistenceStoreFactory_CreateForRepositorySource_MongoDbAndDevTunnelAndInMemory	AgentPersistenceStoreFactoryTests	DevTunnel/MongoDB/in-memory branches are created by the factory with equivalent behavior to the old inline switch.
ToolResourceFactory_CreateMcpServerResolution_UsesMachineUserDefaultsPrecedence	ToolResourceFactoryTests	The extracted MCP tool-resource factory preserves machine > ${USER}/mcp-servers > defaults/mcp-servers precedence (issue #1399).
AgentSessionEntityFactory_CreateEntityData_EscapesFreeTextAndSetsOptionalFields	AgentSessionEntityFactoryTests	The extracted entity factory builds the agent-session document (names, display name, parameter-values, host-profile) with safe JSON escaping (issue #1397).

[JoshuaRowePhantom]

Design & Implementation notes

Commit: 5ca827b (branch fix/1403-1402)

Implemented as the umbrella refactor; #1402 is fixed by construction (single commit references both).

Design

Introduced one host-level composition root, Services/AgentServicesComposition.cs, that produces the complete AgentServices bundle. ComposeHostServices(secretProvider, mcpOAuthOptions) builds the process-wide seed (used by App.axaml.cs), and ComposeSessionServicesAsync(...) reuses it (record with) to add the toolset chain, MCP tool-resource factory, account-upsert service, and current-session context. Both App.axaml.cs and AgentSessionShortcutContext now call this one place, so SecretProvider/McpOAuthOptions can never drift between paths (#1401/#1402 class of bug).

Heavy-lifting moved out of the GUI ViewModel:

• §2 tool-resource composition → Services/ToolResourceFactory.CreateMcpServerResolution(dal, userName, effectiveComputerName). Preserves machine > ${USER}/mcp-servers > defaults/mcp-servers precedence (MCP tool-resource resolver does not search the default user location (${USER}/mcp-servers), so UI-created MCP servers are unresolvable #1399) exactly.
• §3 persistence switch → Services/AgentPersistenceStoreSourceFactory.CreateForRepositorySourceAsync(RepositorySource) with the Web / DevTunnel(reconnecting) / MongoDB / in-memory helpers moved verbatim (GitHub-auth-token resolution + chat-history-provider wiring + DevTunnel reconnection preserved). Kept in the host project because RepositorySource and the DevTunnel services live there.
• §4 entity authoring → data-layer Data.Core/AgentSessionEntityFactory (CreateSessionSimpleName, CreateEntityData). Preserves Agent session names and display names should include human-readable creation date/time and the originating computer name #1397 (local time + computer in display name, sanitized computer + sortable timestamp + session id in entity name, JsonNode-assembled JSON-safe document).

§5 result

AgentSessionShortcutContext now: resolves GUI context → calls the shared composition → calls the persistence-store factory → calls the entity factory → issues UpdateAsync. No MCP wiring, no persistence switch, no JSON authoring.

Files changed

• Added: Services/AgentServicesComposition.cs, Services/AgentPersistenceStoreSourceFactory.cs, Services/ToolResourceFactory.cs, Data.Core/AgentSessionEntityFactory.cs
• Modified: ViewModels/AgentSessionShortcutContext.cs (thin adapter), Services/ApplicationServices.cs (McpOAuthOptions), App.axaml.cs
• Tests: AgentServicesCompositionTests, AgentSessionShortcutContextTests (+4), AgentPersistenceStoreFactoryTests, ToolResourceFactoryTests, AgentSessionEntityFactoryTests, McpTransportFactoryTests (+1)

Tests

Targeted run green: 42/42 (Data.Core.Tests 3, Llm.Core.Tests 16, Phantom.Workspaces.Tests 23).

Deviations

• New factories that must reference RepositorySource / DevTunnel / McpServerEntityToolResourceFactory live in the host Services layer (not Llm.Core) to respect existing project references; the GUI ViewModel no longer knows any of this, satisfying the "out of the GUI" goal.

[JoshuaRowePhantom]

Verification result

Status: ✅ Verified
Commits checked: 5ca827b0 Fix #1403: centralize AgentServices composition root (obviates #1402: thread McpOAuthOptions to session-launch path)

Scope for this issue: the umbrella refactor — single composition root + extractions.

Feature coverage

• Single AgentServices composition root — Phantom.Workspaces/Services/AgentServicesComposition.cs produces the complete bundle (ComposeHostServices for the process-wide seed: SecretProvider + McpOAuthOptions; ComposeSessionServicesAsync reuses it via record with and adds the toolset chain, MCP tool-resource factory, GitHubAccountUpsertService, current-session context, persistence-store override, logger factory). ✔
• Both paths route through it — App.axaml.cs:316 now calls AgentServicesComposition.ComposeHostServices(...) for the app-level AgentChatFactory seed; AgentSessionShortcutContext.CreateAgentServicesAsync calls ComposeSessionServicesAsync. Repo-wide grep for new AgentServices { ... } in the GUI/host finds only MainWindowViewModel.CreateDefaultApplicationServices (headless default, intentionally minimal/null McpOAuthOptions) — no second hand-assembled bundle remains. ✔
• AgentSessionShortcutContext is now a thin adapter — no RepositorySource switch, no MCP tool-resource composition, no entity JSON authoring. The reflection guard test asserts CreateToolResourceFactory/CreateFixedToolMapping/CreateAgentPersistenceStoreAsync/CreateAgentSessionEntityData/CreateSessionObjectSimpleName/SanitizeNameComponent are all gone; confirmed by eyeballing the class (it now only resolves GUI context → calls the factories → issues UpdateAsync). ✔
• MCP tool-resource composition moved out of GUI — Services/ToolResourceFactory.CreateMcpServerResolution(dal, userName, effectiveComputerName). ✔
• Persistence switch moved out of GUI — Services/AgentPersistenceStoreSourceFactory.CreateForRepositorySourceAsync (Web / DevTunnel-reconnecting / MongoDB / in-memory). ✔
• Entity authoring moved to data layer — Phantom.Workspaces.Data.Core/AgentSessionEntityFactory (CreateSessionSimpleName, CreateEntityData). ✔

Behavior-preservation (diffed extracted code against features — moved verbatim)

• MCP tool-resource resolver does not search the default user location (${USER}/mcp-servers), so UI-created MCP servers are unresolvable #1399 precedence preserved: machine > ${USER}/mcp-servers > defaults/mcp-servers, ${USER} binds via the session DAL. Guarded by ToolResourceFactory_CreateMcpServerResolution_UsesMachineUserDefaultsPrecedence. ✔
• Agent session names and display names should include human-readable creation date/time and the originating computer name #1397 entity authoring preserved: local-time + computer display name, sanitized-computer + sortable-timestamp + session-id simple name, JsonNode-assembled JSON-safe document. Guarded by AgentSessionEntityFactory tests (incl. quotes/backslashes round-trip). ✔
• Secret resolution bypassed for MCP key connections and model API keys; remove duplicate sync ResolveApiKey #1398/Secret materialization is gated on the manifest; agent-session launches never resolve ${SECRET:...} (only sessions/definitions should materialize secrets) #1401 SecretProvider supplied on every launch path — ComposeSessionServicesAsync sources it from ComposeHostServices, so the session path can't drop it. Verified same-instance in tests. ✔
• Persistence-store behavior preserved: Web endpoint validation + GitHub-auth-token resolution, DevTunnel reconnecting store, MongoDB chat-history-provider wiring, in-memory fallback — all moved unchanged. ✔

Tests

All 7 expected tests present, deterministic (no Skip/Sleep/Delay/TODO), and asserting the described behavior. Targeted run green (solution built clean):

=== Test Run Summary ===
Executed : 42
Passed   : 42
Failed   : 0

Outstanding gaps

None — all criteria satisfied.

needs-slow-tests applied: the diff moves the persistence-store layer (Web/DevTunnel/MongoDB), so the full suite should run at checkin.

[JoshuaRowePhantom]

Diagnosis: test-host crash on full run is unrelated to this change

Failing/aborting test: AgentFactoryTests.CreateAgentChat_UsesAgentDefinitionChatHistoryTool
Filed/updated bug: #791 (existing, already diagnosed) — #791 (comment)
Why unrelated: All 6176 tests passed (Failed: 0). The host-process crash was the known MongoDB cluster-selection hang that aborts the host after tests report, firing only on FULL runs. The hang stack runs through pre-existing code (AgentFactory -> AgentPersistenceStoreFactory -> MongoDbConnectionBroker -> MongoDB driver cluster selection); none of this branch's changed host-composition files appear in the stack.
Dump archived: dumps\791\testhost_744_20260902T174515_hangdump.dmp

[JoshuaRowePhantom]

Checked in

Commit: 5ca827b
Branch fast-forwarded: features → 5ca827b

Full suite green: 6176 passed, 0 failed. The only host abort was the pre-existing, unrelated known flake #791 (AgentFactoryTests.CreateAgentChat_UsesAgentDefinitionChatHistoryTool MongoDB cluster-selection hang) — confirmed unrelated and archived to dumps/791/.