Uh oh!
There was an error while loading. Please reload this page.
.NET: Add FileAccessProvdider and concurrency fix for FileMemoryProvider - #5583
Conversation
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 90%
✓ Correctness
This PR adds a new FileAccessProvider for shared file access, adds thread safety (SemaphoreSlim write lock + IDisposable) to FileMemoryProvider, reorganizes file store classes into a dedicated FileStore directory, and adds a data processing sample. The code is correct: path traversal protection is properly delegated to StorePaths.NormalizeRelativePath, the write lock correctly serializes save/delete operations with index rebuilds, AsyncLocal-backed CurrentRunContext ensures concurrent tests are safe, and the new provider is a clean, minimal wrapper. No correctness issues found.
✓ Security Reliability
This PR adds a new
FileAccessProviderfor shared file access and improvesFileMemoryProviderwith aSemaphoreSlimwrite lock for thread safety. Security-critical areas look solid: path traversal is blocked byStorePaths.NormalizeRelativePath()(rejects.., absolute paths, drive roots) andFileSystemAgentFileStore.ResolveSafePath()(full-path prefix check). Regex from LLM-sourced tool calls has a 5-second timeout in both store implementations to mitigate ReDoS. TheSemaphoreSlimis properly guarded with try/finally and disposed via the newIDisposableimplementation. No security or reliability issues identified.
✓ Test Coverage
The PR adds a new FileAccessProvider with comprehensive test coverage (577 lines) covering all five tools, constructor validation, path traversal protection, and options. FileMemoryProvider gains IDisposable and a SemaphoreSlim write lock with corresponding thread-safety tests. Test coverage is solid overall with only minor gaps: no test for subdirectory paths through FileAccessProvider tools (e.g., 'reports/summary.md'), and the Dispose test only verifies idempotent dispose without checking post-dispose operation behavior. No blocking issues found.
✓ Design Approach
The change mostly moves the file-store abstraction in a sensible direction, but two design choices look off: the new sample teaches a least-privilege-violating pattern by mounting its bundled input data as a fully mutable store, and the new FileAccessProvider exposes an internally inconsistent contract where agents can write nested paths they cannot later discover via list/search. Both issues are about the chosen abstraction boundary rather than implementation details.
Automated review by westey-m's agents
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Pull request overview
This PR introduces a new FileAccessProvider (plus a harness sample) to give agents persistent/shared file tooling, and adds new AgentFileStore implementations to support file operations. It also updates FileMemoryProvider to address a concurrency issue during writes/index rebuilds.
Changes:
- Added
AgentFileStoreabstraction withInMemoryAgentFileStoreandFileSystemAgentFileStore, plus shared path/glob utilities (StorePaths) and search result models. - Added
FileAccessProvider(+ options) and comprehensive unit tests, along with a new harness sample (Harness_Step03_DataProcessing). - Fixed
FileMemoryProviderconcurrent write/index rebuild behavior via a write lock and added thread-safety tests.
Reviewed changes
Copilot reviewed 11 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| dotnet/src/Microsoft.Agents.AI/Harness/FileStore/AgentFileStore.cs | New base abstraction for agent file operations |
| dotnet/src/Microsoft.Agents.AI/Harness/FileStore/StorePaths.cs | Shared normalization + glob matching helpers |
| dotnet/src/Microsoft.Agents.AI/Harness/FileStore/InMemoryAgentFileStore.cs | In-memory AgentFileStore implementation with search support |
| dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileSystemAgentFileStore.cs | Disk-backed AgentFileStore implementation with traversal protection + search |
| dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileSearchResult.cs | DTO for search results |
| dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileSearchMatch.cs | DTO for per-line matches |
| dotnet/src/Microsoft.Agents.AI/Harness/FileAccess/FileAccessProvider.cs | New provider exposing persistent file tools to agents |
| dotnet/src/Microsoft.Agents.AI/Harness/FileAccess/FileAccessProviderOptions.cs | Options for FileAccessProvider instructions |
| dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/FileMemoryProvider.cs | Concurrency fix: serialize writes/index rebuild; implements IDisposable |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileStore/StorePathsTests.cs | Unit tests for StorePaths normalization/globbing |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileStore/InMemoryAgentFileStoreTests.cs | Unit tests for in-memory store behavior/search |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileStore/FileSystemAgentFileStoreTests.cs | Unit tests for file-system store behavior/search |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileMemory/FileMemoryProviderTests.cs | Added concurrency + dispose tests for FileMemoryProvider |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileAccess/FileAccessProviderTests.cs | Unit tests for FileAccessProvider tools/instructions |
| dotnet/samples/02-agents/Harness/README.md | Registers new Step03 sample in harness index |
| dotnet/samples/02-agents/Harness/Harness_Step03_DataProcessing/Program.cs | New sample wiring FileAccessProvider over a data folder |
| dotnet/samples/02-agents/Harness/Harness_Step03_DataProcessing/README.md | Sample walkthrough and setup |
| dotnet/samples/02-agents/Harness/Harness_Step03_DataProcessing/data/sales.csv | Sample dataset shipped to output |
| dotnet/samples/02-agents/Harness/Harness_Step03_DataProcessing/Harness_Step03_DataProcessing.csproj | New sample project |
| dotnet/agent-framework-dotnet.slnx | Adds the new sample project to the solution |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
0295b4c
into
microsoft:feature-harnessUh oh!
There was an error while loading. Please reload this page.
* .NET: Add a TODO AIContextProvider (microsoft#5233) * Add a TODO AIContextProvider * Add unit tests * Address PR comments * Address PR comments * Fix test after removing one tool * .NET: Add a ModeProvider for managing agent modes (microsoft#5247) * Add a ModeProvider for managing agent modes * Fix typo * Fix typo * Fix typo * Address PR comments * .NET: Add sample to show how to build a harness (microsoft#5268) * Add sample to show how to build a harness * Improve sample * Sample max output tokens and model * Fix encoding * Fix model name in readme * Address PR comments * .NET: Add context window size compaction strategy for harness (microsoft#5304) * Add context window size compaction strategy for harness * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Address PR comments --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * .NET: Add a file memory provider (microsoft#5315) * Add a file memory provider * Address PR comments * Fix review comments. * Add additional unit tests * Addressing PR comments. * .NET: Harness: Improve prompts and add FileSystem store (microsoft#5365) * Harness: Improve prompts and add FileSystem store * Address PR comments * .NET: Harness: Improve path validation (microsoft#5404) * Harness: Improve path validation * Address PR comments * .NET: Add always approve helpers, improve sample and fix bug (microsoft#5451) * Add always approve helpers, improve sample and fix bug * Address PR comments * .NET: Make Todo, Mode and FileMemory providers more configurable (microsoft#5477) * Make Todo, Mode and FileMemory providers more configurable * Address PR comments. * .NET: Add subagents provider and sample (microsoft#5518) * Add subagents provider and sample * Addressing PR comments. * .NET: Harness filememory index plus instructions consistency (microsoft#5540) * Add FileMemoryProvider index and improve instruction consistency * Address PR comments. * Address PR comments * Address PR comments. * Apply suggestion from @rogerbarreto Co-authored-by: Roger Barreto <19890735+rogerbarreto@users.noreply.github.com> --------- Co-authored-by: Roger Barreto <19890735+rogerbarreto@users.noreply.github.com> * .NET: Refactor harness console to be more extensible and easy to understand with better UX (microsoft#5573) * Refactor harness console to be more extensible and easy to understand with better UX. * Fix formatting issues. * Allow multiple clarifications in one response * Address PR comments * .NET: Add FileAccessProvdider and concurrency fix for FileMemoryProvider (microsoft#5583) * Add FileAccessProvdider and concurrency fix for FileMemoryProvider * Address PR comments --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Roger Barreto <19890735+rogerbarreto@users.noreply.github.com>
Motivation and Context
Description
Contribution Checklist