feat: memorysmith_agent_invoke — multi-turn MCP sub-agent tool (Phase 1+2)#53
Closed
TheMasonX wants to merge 14 commits into
Closed
feat: memorysmith_agent_invoke — multi-turn MCP sub-agent tool (Phase 1+2)#53TheMasonX wants to merge 14 commits into
TheMasonX wants to merge 14 commits into
Conversation
…re, scope intersection)
Adds multi-turn sub-agent session tool accessible via MCP.
- New: IGpuSlotScheduler + OllamaGpuSlotScheduler (serial GPU slot semaphore, default=1)
- New: AgentSession entity + IAgentSessionStore + InMemoryAgentSessionStore
- New: AgentSessionService (scope intersection, session lifecycle, filtered catalog)
- New: AgentSessionCleanupService (BackgroundService, 5-min idle expiry)
- Modified: ChatToolCatalog — filtered constructor for scoped sub-agent catalogs;
ChatToolExecutionContext extended with NestingDepth, ParentSessionId, InheritedGpuSlot
(NestingDepth=0 always in Phase 1-2; InheritedGpuSlot reserved for Phase 3)
- Modified: McpController — inject AgentSessionService; wire memorysmith_agent_invoke
and memorysmith_agent_session_end; NestingDepth=0 on DelegateToCatalogAsync context
- Modified: MemorySmithOptions — AgentSessionOptions, MaxParallelOllamaRequests,
MaxConcurrentSessionsPerUser
- Modified: ChatTurnRecord — add ParentSessionId (nullable, backward compatible)
- Modified: Program.cs — register IGpuSlotScheduler, IAgentSessionStore,
AgentSessionService, AgentSessionCleanupService
Phase 3 (internal delegation, AvailableInAgent=true, yield-before-delegate) is
deliberately deferred — see InheritedGpuSlot TODO comments and PHASE3 notes in PR.
Block-merge fixes:
- F1: Replace IEnumerable<IChatProvider> with IServiceScopeFactory in AgentSessionService
(captive scoped-into-singleton dependency — would throw at startup)
- F2+F3: Move per-session SemaphoreSlim into AgentSession; remove _sessionLocks dict
(cleanup data race + lock identity race resolved together)
- F4: Wire GetIdleTimeoutMinutes() and GetMaxConcurrentSessions() to read options
(McpOptions.MaxConcurrentSessionsPerUser and AgentSession.IdleTimeoutMinutes
were defined but silently ignored)
- F5: Replace GetIdleOrExpiredAsync(DateTimeOffset) with GetActiveAndIdleAsync()
(single global cutoff cannot express per-session timeout semantics)
- F6: Log Warning when SensitiveRead tools are excluded + AllowSensitiveRead option
- F7: Validate providerOverride against known provider set
Fix-in-PR items:
- F11: CleanupService calls DeleteAsync after expiry (store no longer grows unboundedly)
- F14: AgentSessionService constructor throws if PersistSessions=true (no store wired)
- F15: RequirePrincipalId returns null instead of 'anonymous'; null rejects session creation
- F16: Add MaxHistoryTurns to AgentSessionOptions; TrimHistoryToMaxTurns in AgentSession
- F17: Return error when scope=custom with empty allowed_tools
- F10: EndSessionAsync now calls DeleteAsync; CleanupService calls DeleteAsync too
Council verdict: confidence 0.89 across 5 seats (Archivist 0.93, Architect 0.87,
Concurrency 0.91, DX 0.93, Skeptic 0.81)
…ryTurns guard, F13 comment) - EndSessionAsync: re-check session.Status after acquiring lock to handle race where cleanup expiry races with explicit close (prevents incorrect false return if store.DeleteAsync is called twice on same session) - TrimHistoryToMaxTurns: clamp maxTurns to [1, 10000] to guard against 0 or extreme config values - CreateSessionAsync: add Phase 2 TODO comment documenting F13 model AllowedRoles bypass as a known deferred spec deviation
…nscript ModeIntent - AdminSessions.razor: /admin/sessions page behind CanAdminMemorySmith; shows all active/idle sessions with scope, turn count, status, timestamps; admin force-close via AgentSessionService.AdminCloseSessionAsync - AgentSession.SystemPromptAddendum: field added; gate checks CanEditMemorySmith + RemoteHardened no-op; injection into model prompt deferred to Phase 3 (requires MemoryChatRequest.SystemPromptAddendum addition to ChatServices.cs) - AgentSessionService: optional IChatTranscriptWriter injection; WriteTranscriptAsync writes ChatTurnRecord with ModeIntent='sub_agent' + ParentSessionId linkage - McpController: system_prompt_addendum parsed and threaded through - docs/PHASE3.md: full Phase 3 scope documentation with code entry points, acceptance criteria, and TODO anchors referencing existing code comments
- Critical DI fix: remove optional parameter default for IChatTranscriptWriter; add NullChatTranscriptWriter as a registered no-op default via TryAddSingleton so the DI container always resolves the interface (optional = null does not work with .NET DI constructor resolution) - High: WriteTranscriptAsync now uses CancellationToken.None instead of the request ct — prevents every transcript from being silently dropped when the HTTP request completes and ct is cancelled - High: Fix ChatUsageSummary property names: InputTokens→PromptTokens, OutputTokens→CompletionTokens, ContextTokens→TotalTokens mapping corrected (ChatUsageSummary record uses Input/Output/Context, not Prompt/Completion/Total)
…t/invoke API - Add AgentTools property to ChatToolCatalog - Add AgentWritesEnabled, AgentWriteAutoAccept, CodeSearchService? CodeSearch params to ChatToolExecutionContext - Add EnabledByDefaultInMcp to ChatToolDescriptor - Add SessionId to MemoryChatRequest for agent session tracing
There was a problem hiding this comment.
Pull request overview
This PR introduces Phase 1+2 infrastructure to expose MemorySmith’s internal chat agent as a multi-turn MCP tool (memorysmith_agent_invoke) with server-managed sessions, plus guardrails for concurrency (GPU slot scheduling), session lifecycle/cleanup, transcript integration (via a default no-op writer), and a Blazor admin surface to inspect/force-close sessions. It fits into the existing McpController tool surface by adding dedicated handlers for session-based agent invocation while reusing MemoryChatAgent for actual inference.
Changes:
- Add agent session orchestration (
AgentSessionService+AgentSession+IAgentSessionStore) with idle cleanup and scope intersection logic. - Add Ollama GPU request serialization via
IGpuSlotScheduler/OllamaGpuSlotSchedulerand new configuration knobs inMemorySmithOptions. - Add admin UI + docs/wiki updates:
/admin/sessions, Phase 3 deferred scope doc, and training venv wiki record.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| MemorySmith.App/Services/Training/NullChatTranscriptWriter.cs | Adds default no-op transcript writer to avoid DI failures when training capture is disabled. |
| MemorySmith.App/Services/Training/ChatTurnRecord.cs | Extends transcript record with ParentSessionId for future delegation linkage. |
| MemorySmith.App/Services/OllamaGpuSlotScheduler.cs | Adds semaphore-based scheduling to serialize/limit Ollama inference concurrency. |
| MemorySmith.App/Services/MemorySmithOptions.cs | Adds AgentSessionOptions and Chat.MaxParallelOllamaRequests configuration fields. |
| MemorySmith.App/Services/IGpuSlotScheduler.cs | Introduces GPU slot scheduler abstraction and null implementation. |
| MemorySmith.App/Services/ChatToolCatalog.cs | Adds filtered catalog constructor + extends execution context for nesting/parent/gpu slot stubs. |
| MemorySmith.App/Services/ChatServices.cs | Adds SessionId to MemoryChatRequest to support session-aware invocation. |
| MemorySmith.App/Services/AgentSessions/InMemoryAgentSessionStore.cs | In-memory ephemeral store for session state. |
| MemorySmith.App/Services/AgentSessions/IAgentSessionStore.cs | Defines persistence contract for agent session storage/cleanup. |
| MemorySmith.App/Services/AgentSessions/AgentSessionService.cs | Core lifecycle/scope enforcement + invocation path + transcript emission. |
| MemorySmith.App/Services/AgentSessions/AgentSessionCleanupService.cs | Background expiration of idle sessions under per-session lock. |
| MemorySmith.App/Services/AgentSessions/AgentSession.cs | Session state container w/ embedded semaphore lock + history pruning. |
| MemorySmith.App/Program.cs | Registers new session services + default transcript writer + hosted cleanup service. |
| MemorySmith.App/Controllers/McpController.cs | Adds MCP tool handlers + schemas for memorysmith_agent_invoke / memorysmith_agent_session_end. |
| MemorySmith.App/Components/Pages/AdminSessions.razor | Adds /admin/sessions page for monitoring and force-closing sessions. |
| docs/PHASE3.md | Adds deferred Phase 3 scope/acceptance criteria doc. |
| Data/Memories/Working/dev-training-venv.md | Adds project-wiki memory record about training venv setup. |
Comment on lines
+39
to
+43
| /// <summary> | ||
| bool AgentWritesEnabled = false, | ||
| bool AgentWriteAutoAccept = false, | ||
| CodeSearchService? CodeSearch = null, | ||
| /// <summary> |
Comment on lines
+116
to
+117
| public IReadOnlyList<ChatToolDescriptor> AgentTools => | ||
| _tools.Values.Where(tool => tool.AvailableInMcp).ToList(); |
Comment on lines
+456
to
+460
| // ChatUsageSummary uses InputTokens/OutputTokens/ContextTokens | ||
| // TurnExecution stores them as PromptTokens/CompletionTokens/TotalTokens | ||
| PromptTokens = response.Usage?.InputTokens, | ||
| CompletionTokens = response.Usage?.OutputTokens, | ||
| TotalTokens = response.Usage?.ContextTokens, |
Comment on lines
+76
to
+78
| private static readonly HashSet<string> KnownProviders = | ||
| new(StringComparer.OrdinalIgnoreCase) { "Ollama", "GitHubCopilot" }; | ||
|
|
Comment on lines
+688
to
+689
| ["model"] = new JsonObject { ["type"] = "string", ["description"] = "Optional Ollama model tag override (e.g. 'qwen3.5:4b')." }, | ||
| ["provider"] = new JsonObject { ["type"] = "string", ["description"] = "Optional provider override (e.g. 'Ollama', 'GitHubCopilot')." }, |
Comment on lines
+3
to
+5
| **Status:** NOT IMPLEMENTED in this PR. Phase 3 is explicitly deferred to a separate PR | ||
| aligned with the Sub-Agent Architecture design work (Design doc: `[[FILE_4fnzo8t5]]`, 2026-05-31). | ||
|
|
Comment on lines
+11
to
+15
| ## Location | ||
| - **Venv path:** `D:\temp\memorysmith-training\.venv` | ||
| - **Python:** `D:\temp\memorysmith-training\.venv\Scripts\python.exe` | ||
| - **Scripts alias:** `$venvPy = "D:\temp\memorysmith-training\.venv\Scripts\python.exe"` | ||
|
|
Branch was force-reset to master tip; restoring all PR work: Phase 1 (core): - New: IGpuSlotScheduler + OllamaGpuSlotScheduler (serial GPU scheduling) - New: AgentSession entity + IAgentSessionStore + InMemoryAgentSessionStore - New: AgentSessionService (scope intersection, GPU scheduling, lifecycle) - New: AgentSessionCleanupService (5-min idle expiry under session lock) - Modified: ChatToolCatalog — filtered constructor, extended ChatToolDescriptor (EnabledByDefaultInMcp), ChatToolExecutionContext (AgentWritesEnabled, AgentWriteAutoAccept, CodeSearch, NestingDepth, ParentSessionId, InheritedGpuSlot), AgentTools property - Modified: ChatServices.cs — SessionId added to MemoryChatRequest - Modified: MemorySmithOptions — AgentSessionOptions, MaxParallelOllamaRequests - Modified: ChatTurnRecord — ParentSessionId field - Modified: McpController — AgentSessionService injection, two new tool handlers - Modified: Program.cs — DI registrations + TryAddSingleton for IChatTranscriptWriter Phase 2 (polish): - New: AdminSessions.razor (/admin/sessions Blazor page) - New: NullChatTranscriptWriter (no-op default for IChatTranscriptWriter) - Modified: AgentSessionService — transcript logging ModeIntent=sub_agent, system_prompt_addendum gate, AdminCloseSessionAsync - New: docs/PHASE3.md — Phase 3 scope documentation Build fixes (from prior agent acb6ffb): - ChatToolCatalog: AgentTools property, EnabledByDefaultInMcp, AgentWritesEnabled, AgentWriteAutoAccept, CodeSearchService? CodeSearch - ChatServices: SessionId parameter on MemoryChatRequest - Program.cs: using Microsoft.Extensions.DependencyInjection.Extensions
Comment on lines
+318
to
+322
| // IChatTranscriptWriter: register NullChatTranscriptWriter as default (no-op when training | ||
| // is disabled). Replace with ChatTranscriptWriter when Training:ChatTranscriptEnabled=true. | ||
| // NullChatTranscriptWriter is registered via TryAddSingleton so an existing registration | ||
| // (e.g. from the training harness wiring) takes precedence. | ||
| builder.Services.TryAddSingleton<IChatTranscriptWriter, NullChatTranscriptWriter>(); |
Comment on lines
+143
to
+147
| // Validate provider override against known registered providers. | ||
| if (!string.IsNullOrEmpty(providerOverride) && !KnownProviders.Contains(providerOverride)) | ||
| return CreateSessionResult.Fail( | ||
| $"Unknown provider '{providerOverride}'. Valid values: {string.Join(", ", KnownProviders)}."); | ||
|
|
Comment on lines
+172
to
+173
| var effectiveToolNames = await ComputeEffectiveScopeAsync( | ||
| requestedScope, customTools, caller, profile, opts, ct); |
Comment on lines
+456
to
+461
| // ChatUsageSummary uses InputTokens/OutputTokens/ContextTokens | ||
| // TurnExecution stores them as PromptTokens/CompletionTokens/TotalTokens | ||
| PromptTokens = response.Usage?.InputTokens, | ||
| CompletionTokens = response.Usage?.OutputTokens, | ||
| TotalTokens = response.Usage?.ContextTokens, | ||
| }, |
Comment on lines
+444
to
+447
| TemplateVersion = "sub-agent-v1", | ||
| ModeIntent = "sub_agent", // ← key: identifies sub-agent turns in transcript | ||
| SystemPromptHash = ChatTranscriptWriter.Sha256Hex(session.SessionId), | ||
| ParentSessionId = session.ParentSessionId, |
Comment on lines
+247
to
+251
| var ended = await _agentSessionService.EndSessionAsync(sessionId, User, cancellationToken); | ||
| return ToolText(ended | ||
| ? $"{{\"closed\":true,\"session_id\":\"{sessionId}\"}}" | ||
| : "{\"finish_reason\":\"session_expired\",\"message\":\"Session not found or already closed.\"}"); | ||
| } |
Comment on lines
+3
to
+5
| **Status:** NOT IMPLEMENTED in this PR. Phase 3 is explicitly deferred to a separate PR | ||
| aligned with the Sub-Agent Architecture design work (Design doc: `[[FILE_4fnzo8t5]]`, 2026-05-31). | ||
|
|
Comment on lines
+11
to
+15
| ## Location | ||
| - **Venv path:** `D:\temp\memorysmith-training\.venv` | ||
| - **Python:** `D:\temp\memorysmith-training\.venv\Scripts\python.exe` | ||
| - **Scripts alias:** `$venvPy = "D:\temp\memorysmith-training\.venv\Scripts\python.exe"` | ||
|
|
Comment on lines
+3
to
+8
| /// <summary> | ||
| /// No-op transcript writer used when training is not configured. | ||
| /// Registered as the default <see cref="IChatTranscriptWriter"/> implementation in Program.cs. | ||
| /// The real <see cref="ChatTranscriptWriter"/> is registered instead when | ||
| /// <c>Training:ChatTranscriptEnabled=true</c> is configured in appsettings. | ||
| /// </summary> |
Comment on lines
+688
to
+690
| ["model"] = new JsonObject { ["type"] = "string", ["description"] = "Optional Ollama model tag override (e.g. 'qwen3.5:4b')." }, | ||
| ["provider"] = new JsonObject { ["type"] = "string", ["description"] = "Optional provider override (e.g. 'Ollama', 'GitHubCopilot')." }, | ||
| ["system_prompt_addendum"] = new JsonObject |
Copilot review fixes: 1. ChatToolCatalog.cs: Fix CS1587 — move inline XML doc comments on positional record parameters to valid <param> tags on the record-level doc comment 2. ChatToolCatalog.cs: Fix AgentTools — change filter from AvailableInMcp (identical to McpTools) to AvailableInChat (excludes page_save/page_delete from agent mode) 3. AgentSessionService.cs: Fix TotalTokens — was ChatUsageSummary.ContextTokens (context window size, not total); now InputTokens + OutputTokens 4. AgentSessionService.cs: Remove GitHubCopilot from KnownProviders — not registered in DI by default; would throw InvalidOperationException at runtime 5. McpController.cs: Update provider schema description to only advertise Ollama 6. docs/PHASE3.md: Replace [[FILE_4fnzo8t5]] workspace placeholder with repo link to docs/Design_AgentAsMCPTool.md (now committed) CI task-validation fixes (Test-TaskRecords.ps1): - Fix tsk-0271-search-tool-consolidation.json: id and key were swapped; id='TSK-0271' → 'tsk-0271-search-tool-consolidation', key='tsk-0271...' → 'TSK-0271' - Renumber tsk-0271-model-context-dropdown-unusable → tsk-0273 (TSK-0273) to resolve duplicate key TSK-0271 introduced by the fine-tuning commit 8a77319 - Renumber tsk-0267-chat-todo-tool → tsk-0274 (TSK-0274) to resolve duplicate key TSK-0267 (tsk-0267-add-api-diagnostics already claims it) NOTE: old duplicate files tsk-0271-model-context-dropdown-unusable.json and tsk-0267-chat-todo-tool.json are deleted in a follow-up delete call (github API requires separate delete operations)
…bered to TSK-0273)
Comment on lines
+3
to
6
| using MemorySmith.App.Services.AgentSessions; | ||
| using MemorySmith.App.Services.Training; | ||
| using Microsoft.Extensions.DependencyInjection; | ||
| using MemorySmith.Core.Indexing; |
Comment on lines
+347
to
+375
| // Build a filtered catalog containing only the session's allowed tools. | ||
| var filteredTools = _toolCatalog.All | ||
| .Where(t => t.AvailableInMcp && session.EffectiveToolNames.Contains(t.Name)) | ||
| .ToList(); | ||
| var filteredCatalog = new ChatToolCatalog(filteredTools); | ||
|
|
||
| // Create a DI scope so we resolve IChatProvider correctly (scoped lifetime). | ||
| using var scope = _scopeFactory.CreateScope(); | ||
| var providers = scope.ServiceProvider.GetServices<IChatProvider>().ToList(); | ||
|
|
||
| var agent = new MemoryChatAgent( | ||
| providers, | ||
| _memories, | ||
| _pages, | ||
| _options, | ||
| currentUser: null, | ||
| toolCatalog: filteredCatalog, | ||
| intentInterceptor: _intentInterceptor); | ||
|
|
||
| // Snapshot history under the lock (we already hold it). | ||
| var historyCopy = session.History.Count > 0 ? [.. session.History] : (IReadOnlyList<ChatMessage>?)null; | ||
|
|
||
| var chatRequest = new MemoryChatRequest( | ||
| Message: message, | ||
| Mode: MemoryChatMode.Chat, | ||
| History: historyCopy, | ||
| Model: session.ModelOverride, | ||
| Provider: session.ProviderOverride, | ||
| SessionId: session.SessionId); |
Comment on lines
+448
to
+452
| Model = new TurnModel(response.Model, response.ProviderName), | ||
| TemplateVersion = "sub-agent-v1", | ||
| ModeIntent = "sub_agent", // ← key: identifies sub-agent turns in transcript | ||
| SystemPromptHash = ChatTranscriptWriter.Sha256Hex(session.SessionId), | ||
| ParentSessionId = session.ParentSessionId, |
Comment on lines
+20
to
+24
| /// <summary> | ||
| /// Returns the count of Active or Idle sessions for a principal. | ||
| /// Used to enforce <c>AgentSession:MaxConcurrentSessionsPerUser</c>. | ||
| /// </summary> | ||
| Task<int> GetActiveCountForPrincipalAsync(string principalId, CancellationToken ct); |
Comment on lines
+673
to
679
| ["scope"] = new JsonObject | ||
| { | ||
| ["type"] = "string", | ||
| ["description"] = "Optional memory status name." | ||
| }, | ||
| ["limit"] = new JsonObject | ||
| { | ||
| ["type"] = "integer", | ||
| ["description"] = "Maximum number of hybrid root results." | ||
| }, | ||
| ["referenceDepth"] = new JsonObject | ||
| { | ||
| ["type"] = "integer", | ||
| ["description"] = "How many levels of references/conflicts to include. Clamped to 0-2." | ||
| ["enum"] = new JsonArray { "read_only", "standard", "full", "custom" }, | ||
| ["default"] = "standard", | ||
| ["description"] = "Tool access scope. read_only/standard: search and fetch only. full: all tools the caller has permission to use. custom: specify allowed_tools." | ||
| }, |
Comment on lines
+688
to
+689
| ["model"] = new JsonObject { ["type"] = "string", ["description"] = "Optional Ollama model tag override (e.g. 'qwen3.5:4b')." }, | ||
| ["provider"] = new JsonObject { ["type"] = "string", ["description"] = "Optional provider override. Currently only 'Ollama' is supported; other values will be rejected. Additional providers can be enabled by registering them as IChatProvider in the app's DI container." }, |
Comment on lines
+11
to
+14
| ## Location | ||
| - **Venv path:** `D:\temp\memorysmith-training\.venv` | ||
| - **Python:** `D:\temp\memorysmith-training\.venv\Scripts\python.exe` | ||
| - **Scripts alias:** `$venvPy = "D:\temp\memorysmith-training\.venv\Scripts\python.exe"` |
Comment on lines
+494
to
+576
| private async Task<IReadOnlyList<string>> ComputeEffectiveScopeAsync( | ||
| string requestedScope, | ||
| IReadOnlyList<string>? customTools, | ||
| ClaimsPrincipal caller, | ||
| string securityProfile, | ||
| MemorySmithOptions opts, | ||
| CancellationToken ct) | ||
| { | ||
| var mcpOptions = opts.Mcp; | ||
| var sessionOptions = opts.AgentSession; | ||
|
|
||
| // Step 1+2: All MCP-available tools filtered by server enable/disable config. | ||
| var enabledSet = _toolCatalog.McpTools | ||
| .Where(t => IsMcpToolEnabled(t, mcpOptions)) | ||
| .ToList(); | ||
|
|
||
| // Step 3: Determine caller permissions. | ||
| var canWrite = (await _authService.AuthorizeAsync( | ||
| caller, null, MemorySmithPolicies.CanEditMemorySmith)).Succeeded; | ||
|
|
||
| // SensitiveRead tools are currently absent from the master catalog. | ||
| // AllowSensitiveRead=true will enable them for sub-agents if/when they are added. | ||
| // A warning is logged when any SensitiveRead tool is excluded so developers know | ||
| // the behavior without having to read this comment. | ||
| var canReadSensitive = sessionOptions.AllowSensitiveRead && | ||
| (await _authService.AuthorizeAsync( | ||
| caller, null, MemorySmithPolicies.CanReadSourceBundle)).Succeeded; | ||
|
|
||
| var sensitiveReadExcluded = enabledSet | ||
| .Where(t => t.Risk == ChatToolRisk.SensitiveRead && !canReadSensitive) | ||
| .Select(t => t.Name) | ||
| .ToList(); | ||
| if (sensitiveReadExcluded.Count > 0) | ||
| { | ||
| _logger.LogWarning( | ||
| "SensitiveRead tools {ToolNames} excluded from sub-agent scope because " + | ||
| "AgentSession:AllowSensitiveRead is false or caller lacks CanReadSourceBundle.", | ||
| string.Join(", ", sensitiveReadExcluded)); | ||
| } | ||
|
|
||
| // Filter by risk tier vs. caller's permissions. | ||
| var callerSet = enabledSet.Where(t => t.Risk switch | ||
| { | ||
| ChatToolRisk.ReadOnly => true, | ||
| ChatToolRisk.SensitiveRead => canReadSensitive, | ||
| ChatToolRisk.Write => canWrite, | ||
| _ => false | ||
| }).ToList(); | ||
|
|
||
| // Step 4: Apply requested scope. | ||
| IEnumerable<ChatToolDescriptor> scopedSet = requestedScope switch | ||
| { | ||
| "read_only" or "standard" => callerSet.Where(t => t.Risk == ChatToolRisk.ReadOnly), | ||
| "full" => callerSet, | ||
| "custom" when customTools is { Count: > 0 } => | ||
| callerSet.Where(t => customTools.Contains(t.Name, StringComparer.OrdinalIgnoreCase)), | ||
| _ => callerSet.Where(t => t.Risk == ChatToolRisk.ReadOnly) | ||
| }; | ||
|
|
||
| // Step 5: Apply SecurityProfile ceiling. | ||
| scopedSet = securityProfile switch | ||
| { | ||
| MemorySmithSecurityProfiles.RemoteHardened => | ||
| scopedSet.Where(t => t.Risk == ChatToolRisk.ReadOnly), | ||
| MemorySmithSecurityProfiles.SecureLocal => | ||
| scopedSet.Where(t => t.Risk != ChatToolRisk.Write), | ||
| _ => scopedSet // LocalDev: no ceiling | ||
| }; | ||
|
|
||
| // Step 6: Self-exclusion — sub-agents can never spawn or manage sessions. | ||
| // TODO (Phase 3): When AvailableInAgent=true is enabled, this unconditional exclusion | ||
| // is the primary anti-recursion guard alongside NestingDepth enforcement. | ||
| var selfExcluded = new HashSet<string>(StringComparer.OrdinalIgnoreCase) | ||
| { | ||
| "memorysmith_agent_invoke", | ||
| "memorysmith_agent_session_end" | ||
| }; | ||
|
|
||
| return scopedSet | ||
| .Where(t => !selfExcluded.Contains(t.Name)) | ||
| .Select(t => t.Name) | ||
| .ToList(); | ||
| } |
Compile fix (root cause of all CI failures): - Program.cs: add `using Microsoft.Extensions.DependencyInjection.Extensions;` (TryAddSingleton extension method requires this namespace — without it the app fails to compile, causing build-and-test and all browser e2e failures) Copilot thread resolutions: - T8/Program.cs: switch from NullChatTranscriptWriter to ChatTranscriptWriter as default IChatTranscriptWriter (ChatTranscriptWriter already no-ops when disabled) - T10/AgentSessionService: guard against empty effective tool set after scope computation — return clear error instead of creating a zero-tool session - T13/McpController: session_end now returns isError=true when not found/closed (was isError=false, making success indistinguishable from failure) - T19/AgentSessionService: tool filter in InvokeCoreAsync changed from AvailableInMcp to AvailableInChat — MemoryChatAgent runs in chat mode and only executes ChatTools; MCP-only tools (page_save, page_delete) would silently fail anyway - T12/T20/AgentSessionService: SystemPromptHash now hashes stable template ID 'sub-agent-chat-v1' instead of the session ID (random GUID) - T16/NullChatTranscriptWriter: updated doc comment to reflect its new role as a test-only override (ChatTranscriptWriter is now the default) - T21/IAgentSessionStore: fix doc comment config path from 'AgentSession:MaxConcurrentSessionsPerUser' to correct 'MemorySmith:Mcp:MaxConcurrentSessionsPerUser' - T22/McpController: scope 'full' description updated to clarify it includes only agent-chat-mode tools (not MCP-only write tools) - T23/McpController: provider schema updated to document that both DI registration AND server allowlist must be updated to enable new providers Data integrity: - Data/Memories/Working/dev-training-venv.md: replace hardcoded machine-specific Windows paths (D:\temp\...) with configurable placeholder and appsettings reference New tasks: - TSK-0275: Add unit tests for AgentSessionService (scope computation, session ownership, cleanup lock, tool filter, empty-scope guard) - TSK-0276: Phase 3 internal agent delegation (yield-before-delegate, AvailableInAgent, nesting depth ceiling, model_profile_id AllowedRoles)
Comment on lines
25
to
+29
| ChatToolRisk Risk, | ||
| bool AvailableInChat, | ||
| bool AvailableInMcp, | ||
| Func<JsonObject, ChatToolExecutionContext, CancellationToken, Task<ChatToolExecutionResult>> Execute); | ||
|
|
||
| Func<JsonObject, ChatToolExecutionContext, CancellationToken, Task<ChatToolExecutionResult>> Execute, | ||
| bool EnabledByDefaultInMcp = true); |
Comment on lines
+371
to
+375
| _pages, | ||
| _options, | ||
| currentUser: null, | ||
| toolCatalog: filteredCatalog, | ||
| intentInterceptor: _intentInterceptor); |
Comment on lines
+583
to
+591
| // Agent session tools (handled directly by McpController, not in ChatToolCatalog). | ||
| array.Add(BuildTool( | ||
| "memorysmith_agent_invoke", | ||
| "Invoke the MemorySmith chat agent as a scoped sub-agent with its own managed context window. On the first call (no session_id), a new multi-turn session is created and the session_id is returned. Include that session_id in subsequent calls to continue the conversation.", | ||
| BuildAgentInvokeSchema())); | ||
| array.Add(BuildTool( | ||
| "memorysmith_agent_session_end", | ||
| "Explicitly close an agent session created by memorysmith_agent_invoke. Frees resources immediately rather than waiting for idle timeout.", | ||
| BuildAgentSessionEndSchema())); |
Comment on lines
+593
to
+601
| private static bool IsMcpToolEnabled(ChatToolDescriptor tool, McpOptions options) | ||
| { | ||
| if (options.DisabledTools.Contains(tool.Name, StringComparer.OrdinalIgnoreCase)) | ||
| return false; | ||
| if (options.EnabledTools.Count > 0 && | ||
| !options.EnabledTools.Contains(tool.Name, StringComparer.OrdinalIgnoreCase)) | ||
| return false; | ||
| return true; | ||
| } |
Unit tests (TSK-0275): - MemorySmith.Tests/AgentSessionTests.cs: 16 tests covering AgentTools filter, filtered constructor, EnabledByDefaultInMcp, AgentSession embedded lock, history trimming, InMemoryAgentSessionStore CRUD, AgentSessionService scope computation (read_only, custom+empty, disabled-all), session ownership (cross-principal returns not-found equivalent), double EndSession, expired session resume, unknown provider, null principal ChatToolCatalog fixes: - Restore master 10-tool catalog as base (was incorrectly replaced with a different version that missed the correct tool registrations) - Add EnabledByDefaultInMcp=false explicitly on page_save and page_delete (Write-tier tools default-off per governance design) - Add filtered constructor, AgentTools (AvailableInChat filter), extended ChatToolExecutionContext, ChatToolDescriptor.EnabledByDefaultInMcp MCP governance (Threads T4sDr, T4sEa, T4sEn): - AgentSessionService.IsMcpToolEnabled: fix semantics — EnabledTools is now additive opt-in on top of EnabledByDefaultInMcp baseline, not a global allowlist - McpController: memorysmith_agent_invoke and memorysmith_agent_session_end now subject to governance (IsAgentToolEnabled checks EnabledTools/DisabledTools/ EnabledByDefaultInMcp=false default). Operator must add them to EnabledTools before they become callable. tools/list only shows them when enabled. New tasks: - TSK-0277: Implement full ChatToolCatalog (source_bundle, task write tools, memory write tools, code search tools) to make pre-existing test failures pass (ChatToolCatalogAndInterceptTests.cs expects 20+ tools; master only has 10)
- AgentSessionService.ComputeEffectiveScopeAsync: change step 1 catalog baseline from McpTools to McpTools∩AvailableInChat. Sub-agents run in MemoryChatMode.Chat and can only execute ChatTools; storing MCP-only write tools in EffectiveToolNames produced misleading audit logs. This also aligns stored scope with what InvokeCoreAsync actually executes. - TSK-0275: status updated to Done (16 unit tests delivered in this PR) - TSK-0278: created for SqliteAgentSessionStore (SQLite-backed session persistence) PageAccessLevels.CanView(null user) confirmed safe: null principal correctly treated as unauthenticated — sub-agents access only Anonymous-role pages.
Comment on lines
+368
to
+375
| var agent = new MemoryChatAgent( | ||
| providers, | ||
| _memories, | ||
| _pages, | ||
| _options, | ||
| currentUser: null, | ||
| toolCatalog: filteredCatalog, | ||
| intentInterceptor: _intentInterceptor); |
Comment on lines
+562
to
+570
| // Step 4: Apply requested scope. | ||
| IEnumerable<ChatToolDescriptor> scopedSet = requestedScope switch | ||
| { | ||
| "read_only" or "standard" => callerSet.Where(t => t.Risk == ChatToolRisk.ReadOnly), | ||
| "full" => callerSet, | ||
| "custom" when customTools is { Count: > 0 } => | ||
| callerSet.Where(t => customTools.Contains(t.Name, StringComparer.OrdinalIgnoreCase)), | ||
| _ => callerSet.Where(t => t.Risk == ChatToolRisk.ReadOnly) | ||
| }; |
Comment on lines
+93
to
+95
| private readonly ILogger<AgentSessionService> _logger; | ||
| private readonly Training.IChatTranscriptWriter _transcriptWriter; // registered via Program.cs; NullChatTranscriptWriter is the default | ||
|
|
Comment on lines
+268
to
+273
| if (session.Status is AgentSessionStatus.Expired or AgentSessionStatus.Closed) | ||
| { | ||
| return new AgentInvokeResult( | ||
| session.SessionId, session.TurnCount, string.Empty, [], | ||
| 0, "session_expired", null); | ||
| } |
Comment on lines
+622
to
+630
| /// <summary> | ||
| /// Returns the caller's principal identifier, or null if the caller is unauthenticated | ||
| /// or has no NameIdentifier claim. A null return causes session creation to be rejected — | ||
| /// anonymous callers cannot create sessions to prevent namespace collision. | ||
| /// </summary> | ||
| private static string? RequirePrincipalId(ClaimsPrincipal caller) => | ||
| caller.FindFirstValue(System.Security.Claims.ClaimTypes.NameIdentifier) | ||
| ?? caller.Identity?.Name; | ||
|
|
TheMasonX
added a commit
that referenced
this pull request
Jun 7, 2026
Rebased from PR #53 (feature/agent-mcp-tool-invoke) onto current master. Preserves all 22 master tools + adds agent session infrastructure. New: AgentSessionService, GPU slot scheduling, session lifecycle, scoped tool catalogs, admin UI, Phase 3 context fields. Fixes: missing using directive for TryAddSingleton. Excludes deprecated tools (semantic_search, unified_search).
4 tasks
Owner
Author
|
Superseded by #54 — rebased onto current master with conflict resolution. The original branch had merge conflicts and a stale ChatToolCatalog.cs (10 tools vs master's 22). PR #54 preserves all 22 master tools, fixes the build-breaking missing using directive, and applies only the new agent session infrastructure on top of master's current state. Council review approved the merge strategy at 78-81% confidence. |
TheMasonX
added a commit
that referenced
this pull request
Jun 7, 2026
Add memorysmith_agent_invoke and memorysmith_agent_session_end MCP tools for multi-turn sub-agent conversations with GPU slot scheduling, session lifecycle, scope intersection, and admin UI. Rebased from PR #53 onto current master. Preserves all 22 master tools. Fixes missing using directive. Adds Phase 3 context fields. New files: AgentSessionService, AgentSession, AgentSessionCleanupService, InMemoryAgentSessionStore, IGpuSlotScheduler, OllamaGpuSlotScheduler, NullChatTranscriptWriter, AdminSessions.razor, AgentSessionTests. Deferred: TSK-0275 (unit tests), TSK-0276 (Phase 3), TSK-0277 (catalog completion), TSK-0278 (SQLite store). Supersedes #53.
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.
Summary
Exposes MemorySmith's internal chat agent as a multi-turn MCP tool (
memorysmith_agent_invoke), allowing external callers (Claude, Cursor, etc.) and — in a future phase — internal Athena sessions to delegate research and collation tasks to a scoped sub-agent with its own context window.Design doc:
docs/PHASE3.md| Design_AgentAsMCPTool.md (workspace)What's included
Phase 1 — Core infrastructure
memorysmith_agent_invokeMCP tool (Risk=Write;CanEditMemorySmithrequired) and companionmemorysmith_agent_session_endsession_id; subsequent calls continue the conversationIGpuSlotScheduler/OllamaGpuSlotScheduler: serializes Ollama access viaSemaphoreSlim(1,1)to prevent VRAM exhaustion on single-GPU hostsAgentSessionService: scope intersection (requested ∩ server-enabled ∩ caller-perms ∩ non-self-referential), SecurityProfile-driven defaults, session lifecycleIAgentSessionStore/InMemoryAgentSessionStore: in-memory ephemeral defaultAgentSessionCleanupService: BackgroundService, 5-min idle expiry under session lockChatToolCatalogfiltered constructor:new ChatToolCatalog(IEnumerable<ChatToolDescriptor>)for scoped sub-agent catalogsChatToolExecutionContextextended withNestingDepth,ParentSessionId,InheritedGpuSlot(Phase 3 stubs)Phase 2 — Polish
/admin/sessionsBlazor admin page: view all active sessions, force-close viaAdminCloseSessionAsyncsystem_prompt_addendum: stored on session withCanEditMemorySmithgate + RemoteHardened no-op; injection into model prompt deferred to Phase 3IChatTranscriptWriterintegration: sub-agent turns written withModeIntent="sub_agent"+ParentSessionIdlinkage;NullChatTranscriptWriterregistered as defaultdocs/PHASE3.md: full Phase 3 scope document with acceptance criteria and TODO anchorsCouncil review summary
This PR was reviewed by a 6-seat LLM council (
.github/skills/council/SKILL.md) across two phases with iterative fix cycles:Phase 1 council — 29 findings; 7 block-merge fixed:
IServiceScopeFactoryAgentSessionobjectAgentSessionCleanupServiceacquires session lock before mutationGetIdleTimeoutMinutesandGetMaxConcurrentSessionsnow read fromAgentSessionOptions/McpOptionsGetIdleOrExpiredAsynccutoff semantics → replaced withGetActiveAndIdleAsyncAllowSensitiveReadoption +LogWarningKnownProvidersallowlistPhase 2 council — 3 critical/high fixed:
NullChatTranscriptWriter+TryAddSingletonWriteTranscriptAsyncused requestct→ changed toCancellationToken.NoneChatUsageSummaryproperty names wrong → corrected toInputTokens/OutputTokens/ContextTokensPhase 3 — NOT in this PR
Phase 3 (internal delegation: Athena → sub-agent) is explicitly deferred. See
docs/PHASE3.mdfor full scope.Key Phase 3 entry points already stubbed in this code:
ChatToolExecutionContext.InheritedGpuSlot— TODO comment at definitionAgentSession.NestingDepth— ceiling enforcement TODO inCreateSessionAsyncAgentSessionService.ComputeEffectiveScopeAsync— self-exclusion noteAvailableInAgent=falseonmemorysmith_agent_invoke(no structural field yet — guarded via scope computation)Test plan
InvalidOperationExceptionwithValidateScopes=true(Development environment)POST /mcpwith{"method":"tools/call","params":{"name":"memorysmith_agent_invoke","arguments":{"message":"search for X"}}}returnssession_idand agent responsesession_idcontinues the conversation (history preserved)scope=customwith emptyallowed_toolsreturns errorread_onlyscope cannot executememorysmith_page_save/admin/sessionssees active sessions and can force-close oneIdleTimeoutMinutes(cleanup service)🤖 Generated with Claude Code