.NET: Harness filememory index plus instructions consistency#5540
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR expands the .NET “Harness” feature set by adding file-memory indexing (via memories.md) and introducing/standardizing several built-in harness providers (todo, agent mode, sub-agents, tool approval), along with tests and new samples that demonstrate the workflow.
Changes:
- Add FileMemory enhancements: safe path/glob helpers, file stores, search/list APIs, and a generated memory index injected into context.
- Add harness providers and persisted state for Todo, AgentMode, SubAgents, and ToolApproval (plus JSON source-gen registrations).
- Add comprehensive unit tests and new Harness samples + solution references.
Reviewed changes
Copilot reviewed 61 out of 61 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/ToolApproval/ToolApprovalRuleTests.cs | Adds unit tests for tool approval rule defaults + JSON round-trips. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/ToolApproval/ToolApprovalAgentBuilderExtensionsTests.cs | Tests builder extension behavior and null-guard. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/ToolApproval/AlwaysApproveToolApprovalResponseContentTests.cs | Tests always-approve response wrappers and extension methods. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/Todo/TodoProviderTests.cs | Tests todo tool behaviors and session persistence. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileMemory/StorePathsTests.cs | Tests path normalization and glob matching helpers. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileMemory/InMemoryAgentFileStoreTests.cs | Tests in-memory store CRUD, listing, and regex/glob search behavior. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileMemory/FileSystemAgentFileStoreTests.cs | Tests filesystem-backed store safety and file operations. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/AgentMode/AgentModeProviderTests.cs | Tests mode tools, options validation, and notification injection. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/Compaction/ContextWindowCompactionStrategyTests.cs | Adds coverage for ContextWindowCompactionStrategy constructor + compaction behavior. |
| dotnet/src/Microsoft.Agents.AI/Microsoft.Agents.AI.csproj | Adds globbing package dependency for file filtering. |
| dotnet/src/Microsoft.Agents.AI/Harness/ToolApproval/ToolApprovalState.cs | Adds persisted state for tool approval queues/rules. |
| dotnet/src/Microsoft.Agents.AI/Harness/ToolApproval/ToolApprovalRule.cs | Adds tool approval rule model with JSON names. |
| dotnet/src/Microsoft.Agents.AI/Harness/ToolApproval/ToolApprovalRequestContentExtensions.cs | Adds helper APIs to create “always approve” responses. |
| dotnet/src/Microsoft.Agents.AI/Harness/ToolApproval/ToolApprovalAgentBuilderExtensions.cs | Adds builder extension for tool-approval middleware. |
| dotnet/src/Microsoft.Agents.AI/Harness/ToolApproval/AlwaysApproveToolApprovalResponseContent.cs | Adds wrapper AIContent carrying “always approve” flags. |
| dotnet/src/Microsoft.Agents.AI/Harness/Todo/TodoState.cs | Adds persisted todo list state model. |
| dotnet/src/Microsoft.Agents.AI/Harness/Todo/TodoProviderOptions.cs | Adds configurable todo instructions option. |
| dotnet/src/Microsoft.Agents.AI/Harness/Todo/TodoProvider.cs | Adds todo tools, instructions, and session-backed state management. |
| dotnet/src/Microsoft.Agents.AI/Harness/Todo/TodoItemInput.cs | Adds tool input DTO for creating todo items. |
| dotnet/src/Microsoft.Agents.AI/Harness/Todo/TodoItem.cs | Adds public todo item model for tool results/persistence. |
| dotnet/src/Microsoft.Agents.AI/Harness/SubAgents/SubTaskStatus.cs | Adds enum representing sub-task lifecycle. |
| dotnet/src/Microsoft.Agents.AI/Harness/SubAgents/SubTaskInfo.cs | Adds model for sub-task metadata/results. |
| dotnet/src/Microsoft.Agents.AI/Harness/SubAgents/SubAgentsProviderOptions.cs | Adds customization options for sub-agent instructions/listing. |
| dotnet/src/Microsoft.Agents.AI/Harness/SubAgents/SubAgentsProvider.cs | Adds sub-agent delegation tools and runtime tracking. |
| dotnet/src/Microsoft.Agents.AI/Harness/SubAgents/SubAgentState.cs | Adds persisted sub-task state. |
| dotnet/src/Microsoft.Agents.AI/Harness/SubAgents/SubAgentRuntimeState.cs | Adds non-serializable runtime refs for in-flight tasks/sessions. |
| dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/StorePaths.cs | Adds path normalization + glob utilities shared across stores/provider. |
| dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/InMemoryAgentFileStore.cs | Adds in-memory file store implementation with regex+glob search. |
| dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/FileSystemAgentFileStore.cs | Adds safe filesystem-backed file store under a root directory. |
| dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/FileSearchResult.cs | Adds search result DTO for file content searching. |
| dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/FileSearchMatch.cs | Adds matching-line DTO with line numbers. |
| dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/FileMemoryState.cs | Adds session state for provider working folder. |
| dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/FileMemoryProviderOptions.cs | Adds configurable file-memory instructions option. |
| dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/FileMemoryProvider.cs | Adds file memory tools + auto-generated memory index injection. |
| dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/FileListEntry.cs | Adds file listing DTO with optional description. |
| dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/AgentFileStore.cs | Adds base abstraction for file stores. |
| dotnet/src/Microsoft.Agents.AI/Harness/AgentMode/AgentModeState.cs | Adds session state for current mode + external-change notification. |
| dotnet/src/Microsoft.Agents.AI/Harness/AgentMode/AgentModeProviderOptions.cs | Adds options model for mode provider and custom mode list. |
| dotnet/src/Microsoft.Agents.AI/Harness/AgentMode/AgentModeProvider.cs | Adds mode tools, instruction injection, and external-change notifications. |
| dotnet/src/Microsoft.Agents.AI/Compaction/ContextWindowCompactionStrategy.cs | Adds compaction strategy derived from context window + output tokens. |
| dotnet/src/Microsoft.Agents.AI/ChatClient/PerServiceCallChatHistoryPersistingChatClient.cs | Clones streaming response updates before storing them. |
| dotnet/src/Microsoft.Agents.AI/AgentJsonUtilities.cs | Registers harness types for JSON source generation. |
| dotnet/samples/02-agents/README.md | Adds Harness sample link. |
| dotnet/samples/02-agents/Harness/README.md | Adds harness samples landing page. |
| dotnet/samples/02-agents/Harness/Harness_Step02_Research_WithSubAgents/README.md | Documents sub-agent demo sample. |
| dotnet/samples/02-agents/Harness/Harness_Step02_Research_WithSubAgents/Program.cs | Implements sub-agent orchestration demo using SubAgentsProvider. |
| dotnet/samples/02-agents/Harness/Harness_Step02_Research_WithSubAgents/Harness_Step02_Research_WithSubAgents.csproj | Adds project for sub-agent demo sample. |
| dotnet/samples/02-agents/Harness/Harness_Step01_Research/WebBrowsingTool.cs | Adds local web browsing tool (HTML→Markdown) for research sample. |
| dotnet/samples/02-agents/Harness/Harness_Step01_Research/README.md | Documents research harness sample workflow/usage. |
| dotnet/samples/02-agents/Harness/Harness_Step01_Research/Program.cs | Implements research sample using Todo/Mode/FileMemory/ToolApproval+Compaction. |
| dotnet/samples/02-agents/Harness/Harness_Step01_Research/Harness_Step01_Research.csproj | Adds project for research harness sample. |
| dotnet/samples/02-agents/Harness/Harness_Shared_Console/ToolCallFormatter.cs | Adds console formatter for known tool calls. |
| dotnet/samples/02-agents/Harness/Harness_Shared_Console/Spinner.cs | Adds console spinner utility. |
| dotnet/samples/02-agents/Harness/Harness_Shared_Console/Harness_Shared_Console.csproj | Adds shared console helper project for harness samples. |
| dotnet/samples/02-agents/Harness/Harness_Shared_Console/HarnessConsole.cs | Adds interactive console loop with approvals, mode colors, usage display. |
| dotnet/agent-framework-dotnet.slnx | Includes new harness sample projects in solution. |
| dotnet/Directory.Packages.props | Pins Microsoft.Extensions.FileSystemGlobbing version. |
Comments suppressed due to low confidence (2)
dotnet/src/Microsoft.Agents.AI/Harness/SubAgents/SubAgentsProvider.cs:1
- The
SubAgents_StartTask/SubAgents_WaitForFirstCompletiontool descriptions claim they return IDs, but the implementations return formatted strings. This makes tool outputs harder for agents to consume reliably. Consider returning a structured result (e.g., aninttaskId or an object containing{ taskId, status }) or update the tool descriptions to match the actual return type.
dotnet/src/Microsoft.Agents.AI/Harness/SubAgents/SubAgentsProvider.cs:1 - When clearing a completed task, the sub-task
AgentSessionis removed fromSubTaskSessionsbut not disposed. IfAgentSessionholds resources (e.g., network handles, buffers, timers), this can leak across long-running sessions. Consider disposing the session when clearing (e.g., if it implementsIDisposable/IAsyncDisposable) before removing the reference.
There was a problem hiding this comment.
Automated Code Review
Reviewers: 3 | Confidence: 89%
✓ Correctness
No actionable issues found in this dimension.
✓ Security Reliability
The library code is well-structured with proper input validation (path traversal protection in FileSystemAgentFileStore, threshold validation in ContextWindowCompactionStrategy, mode validation in AgentModeProvider). The WebBrowsingTool sample lacks SSRF protections—it accepts any absolute URI without restricting schemes or blocking internal/cloud-metadata IP ranges. While this is sample code, it demonstrates a pattern that users are likely to copy. The .Clone() fix in PerServiceCallChatHistoryPersistingChatClient is a sound reliability improvement.
✗ Design Approach
First,
AgentModeProvidermodels an out-of-band/modeswitch by injecting a syntheticChatRole.Usermessage, which turns control-plane state into durable conversation history instead of a transient instruction. Second,AgentFileStoreclaims to be backend-neutral but hardcodesMicrosoft.Extensions.FileSystemGlobbing.Matchersemantics into the abstract search contract, leaking local-filesystem behavior into every future store implementation.FileMemory_SearchFilescurrently exposes the provider’s own implementation artifacts (memories.mdand*_description.md) even though the rest of the provider deliberately hides them, which leaks internal storage details into the model-facing API. Separately,SubAgents_ContinueTaskpromises preserved conversational context, but the implementation keeps child sessions only in a non-serializable runtime map, so that capability disappears after normal session persistence/restoration even though the framework already supports serializing agent sessions. The new tool-approval middleware takes the right general direction, but its rule extraction/matching is hard-coded toFunctionCallContent, while the existing workflow layer already emitsToolApprovalRequestContentfor MCP tool calls as well. That makes the "always approve" feature an overly narrow fix: it appears generic in the API surface, but in practice it will never persist or replay approvals for MCP-backed tools. The new tests are generally aligned with the implementation, but one test locks in a path contract that is too permissive for a helper shared by both the in-memory and filesystem-backed stores. Allowing whitespace-only file names inStorePaths.NormalizeRelativePathmakes the abstractAgentFileStorepath model depend on host filesystem quirks instead of defining a portable logical path contract. The new tests are otherwise comprehensive, but they lock in a brittle design for "always approve with arguments": matching is based on exact serialized JSON text rather than semantic argument equality. That makes approvals depend on incidental formatting details like object property order, which is a fragile contract for persisted approval rules.
Flagged Issues
-
ToolApprovalAgentonly records and matches standing approval rules forFunctionCallContent, butInvokeMcpToolExecutoralready createsToolApprovalRequestContentaroundMcpServerToolCallContent(InvokeMcpToolExecutor.cs:79-90). "Always approve" silently does nothing for MCP approvals, so the middleware does not actually solve tool approval generically. Extract a common tool identity/arguments shape from each supported tool-call content type and match rules against that abstraction. -
StorePathsTestscodifies whitespace-only file names as valid even thoughStorePaths.NormalizeRelativePathis shared byFileSystemAgentFileStore(StorePaths.cs:31-72, FileSystemAgentFileStore.cs:239-244). This bakes host-specific filesystem quirks into the common path abstraction instead of rejecting non-portable paths up front. - ToolApprovalAgentTests.cs:627-633 codifies raw JSON-string matching for rule arguments. The implementation compares serialized argument text exactly (ToolApprovalAgent.cs:668-731), so semantically equivalent object arguments with different property order will fail to match. Persisted approval rules should use normalized/structural argument equality instead of serialized-text identity.
Automated review by westey-m's agents
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 92%
✓ Correctness
The PR refactors several providers to use placeholder-based instruction templates, adds a memory index to FileMemoryProvider, and fixes several issues from prior review. The main correctness concern is in SubAgentsProvider: switching from string concatenation to
Replace("{sub_agents}", ...)silently drops the agent list when custom Instructions don't contain the placeholder. The existing doc on SubAgentsProviderOptions.Instructions (line 22, not updated by this PR) still promises "The agent list is always appended after the instructions regardless of this setting," which is now incorrect.
✓ Security Reliability
The PR improves instruction templating, adds a memory index to FileMemoryProvider, and addresses several previously flaged issues. Three concerns remain: (1) SaveFileAsync doesn't reject internal file names like 'memories.md', causing silent data loss when RebuildMemoryIndexAsync immediately overwrites the user's content; (2) the memory index is injected as a synthetic ChatRole.User message, amplifying prompt injection risk from previously stored content; (3) SubAgentsProvider's switch from string concatenation to .Replace("{sub_agents}", ...) silently drops the agent list when custom instructions lack the placeholder.
✗ Test Coverage
The PR adds significant test coverage for the new memory index feature (7 new tests in FileMemoryProviderTests.cs) and updates existing SubAgentsProvider tests for the placeholder-based instruction mechanism. However, two new code paths introduced in this PR lack any test coverage: (1) the
SubTaskStatus.Lostguard inContinueTaskand (2) theSearchFilesAsyncinternal-file filtering. The existingListFiles_HidesMemoryIndexAsynctest verifies listing hidesmemories.md, but no analogous test exists for search. The AgentModeProvider's new template-basedBuildInstructionsmethod is only shallowly tested (existing test checksContains("plan")which would pass with both old and new implementations).
✓ Design Approach
I found two design-level issues that should be addressed before merging. The new file-memory index is being injected as a synthetic user message, which makes a provider-generated listing of saved files part of durable chat history instead of transient context. Separately, the sub-agent instruction templating changes a previously append-based public contract into a magic-placeholder contract without a compatibility fallback, so existing custom instructions will silently lose the available-agent list.
Flagged Issues
- New
SubTaskStatus.Losterror path in ContinueTask (SubAgentsProvider.cs:386) has no test. Existing ContinueTask tests cover Running and nonexistent task but not Lost. Add aContinueTask_LostTask_ReturnsErrorAsynctest.
Automated review by westey-m's agents
Co-authored-by: Roger Barreto <19890735+rogerbarreto@users.noreply.github.com>
Motivation and Context
Description
Contribution Checklist