Add fileaccess harness context provider with shared-folder file tools rooted at a directory - #643
Conversation
64a5a47 to
86fa0f1
Compare
This comment has been minimized.
This comment has been minimized.
86fa0f1 to
596246b
Compare
Introduce a self-contained agent/harness/fileaccess package that mirrors the .NET FileAccessProvider. It registers a context provider that injects file tools scoped to a caller-granted root directory: file_access_read_file, file_access_save_file, file_access_list_files, file_access_list_subdirectories, file_access_search_files, and file_access_delete_file. The root comes from Options.RootDir (not session state). A local-filesystem store constrains every operation to the root, rejecting absolute paths and ".." traversal via filepath.Clean plus a prefix check. Options.ReadOnly omits the save and delete tools to match the .NET read-only shipping mode.
596246b to
f6be276
Compare
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Adds a new Go harness context provider (agent/harness/fileaccess) that injects shared-folder file tools (read/save/list/search/delete) into agent invocations, intended to mirror the .NET FileAccessProvider and support a read-only mode.
Changes:
- Introduces
fileaccess.Providerwith tool injection + default/read-only instructions and a local filesystem-backed store rooted atOptions.RootDir. - Implements six file tools (
file_access_*) with path resolution intended to constrain access to the configured root directory, plus read-only mode by omitting mutating tools. - Adds black-box tests validating tool exposure, read-only behavior, round-trip save/read, list/search semantics, delete, and basic
../absolute-path escape rejection.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| agent/harness/fileaccess/fileaccess.go | New file-access provider and local filesystem store for shared-folder tool operations. |
| agent/harness/fileaccess/fileaccess_test.go | New black-box tests covering tool presence/behavior, basic path escape rejection, and core operations. |
Suppressed comments (1)
agent/harness/fileaccess/fileaccess.go:283
- Path containment checks do not account for symlinks inside the root. For example, if the shared folder contains a symlink directory like "link" -> "/tmp", calling save_file with path "link/outside.txt" will pass resolve() (it stays under root textually) but os.MkdirAll/os.WriteFile will follow the symlink and write outside the root. The same issue applies to read/delete/search for symlink files.
func (s *store) SaveFile(rel, content string) error {
full, err := s.resolve(rel)
if err != nil {
return err
}
| // All operations are constrained to the configured root directory. Paths are | ||
| // resolved relative to the root and any attempt to escape it (via "..", an | ||
| // absolute path, or symlink-style traversal in the supplied name) is rejected. | ||
| // |
| full := filepath.Clean(filepath.Join(s.root, rel)) | ||
| if full != s.root && !strings.HasPrefix(full, s.root+string(os.PathSeparator)) { | ||
| return "", fmt.Errorf("path %q escapes the shared folder root", rel) | ||
| } | ||
| return full, nil |
This comment has been minimized.
This comment has been minimized.
|
PratikDhanave (@PratikDhanave) can you resolve the parity gaps? |
github-actions
Bot
added
area:agent
Cross-repo parity review — PR #643Upstream reference: This PR ports the .NET Issues found
What is aligned
Because public exported APIs have changed, the
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Generated by Go API Consistency Review Agent · sonnet46 · 39.6 AIC · ⌖ 5 AIC · ⊞ 6K
| // Package fileaccess provides a context provider that gives agents file tools | ||
| // for reading and writing files inside a single caller-granted directory (a | ||
| // "shared folder"). It mirrors the .NET FileAccessProvider, exposing the same | ||
| // set of tools: file_access_read_file, file_access_save_file, |
There was a problem hiding this comment.
Parity issue: Tool names diverge from .NET FileAccessProvider
The Go package doc comments (and the actual tool registrations) use different tool names than the .NET FileAccessProvider ships:
| Go name | .NET name |
|---|---|
file_access_read_file |
file_access_read |
file_access_save_file |
file_access_write |
file_access_list_files |
file_access_ls |
file_access_list_subdirectories |
(no separate tool; .NET's file_access_ls returns both files and dirs) |
file_access_search_files |
file_access_grep |
file_access_delete_file |
file_access_delete |
Ref: dotnet/src/Microsoft.Agents.AI/Harness/FileAccess/FileAccessProvider.cs — WriteToolName, ReadFileToolName, LsToolName, GrepToolName, DeleteFileToolName.
Using different tool names means prompts, instructions, and tests written for one SDK will not transfer to the other. Please align with the upstream .NET names (or document a justified divergence).
| // file_access_delete_file tools so the agent can only read. | ||
| ReadOnly bool | ||
|
|
||
| // Instructions overrides the default instructions provided to the agent. |
There was a problem hiding this comment.
Parity issue: Missing file_access_replace and file_access_replace_lines tools
The .NET FileAccessProvider ships two additional tools not present in this Go package:
file_access_replace— replaces occurrences of a substring within a file (avoids full rewrites)file_access_replace_lines— replaces whole lines within a file
These are write-mode tools omitted when DisableWriteTools is set.
Ref: dotnet/src/Microsoft.Agents.AI/Harness/FileAccess/FileAccessProvider.cs — ReplaceToolName, ReplaceLinesToolName.
If these are intentionally deferred, please note that in the PR description and/or add a TODO; otherwise the Go provider is functionally incomplete relative to .NET.
| - Use file_access_list_files to list the files directly inside a folder. | ||
| - Use file_access_list_subdirectories to list the sub-folders directly inside a folder. | ||
| - Use file_access_search_files to find files whose contents match a regular expression.` | ||
|
|
There was a problem hiding this comment.
Parity issue: Options diverges from .NET FileAccessProviderOptions
The Go Options struct uses ReadOnly bool while the .NET equivalent uses DisableWriteTools bool. These are semantically equivalent, but the name divergence means documentation and cross-SDK guidance will be inconsistent. Please use DisableWriteTools (or add it as an alias) to match the upstream name.
Also, the .NET options expose two additional fields that Go omits:
DisableReadOnlyToolApproval bool— disables the approval requirement for read-only toolsDisableWriteToolApproval bool— disables the approval requirement for write tools
And the upstream type is marked [Experimental]; Go may want an equivalent //go:build constraint or a package-level note.
Ref: dotnet/src/Microsoft.Agents.AI/Harness/FileAccess/FileAccessProviderOptions.cs.
| store *store | ||
| readOnly bool | ||
| instructions string | ||
| } |
There was a problem hiding this comment.
Parity issue: Tool-approval integration is absent
The .NET FileAccessProvider ships two static auto-approval rules as first-class public API:
FileAccessProvider.ReadOnlyToolsAutoApprovalRule— auto-approvesfile_access_read,file_access_ls,file_access_grepFileAccessProvider.AllToolsAutoApprovalRule— auto-approves all seven tools
These are intended to be registered with the ToolApprovalAgent harness, and the PR description acknowledges the .NET read-only mode uses this pattern. Go's sibling toolapproval harness package presumably supports the same hook. Please either:
- Expose equivalent exported
AutoApprovalRulevalues in this package so callers can wire them into the approval harness, mirroring the .NET public contract, or - Document explicitly why this is deferred and what callers should do instead.
Ref: dotnet/src/Microsoft.Agents.AI/Harness/FileAccess/FileAccessProvider.cs — ReadOnlyToolsAutoApprovalRule, AllToolsAutoApprovalRule.
github-actions
Bot
added
failed-auto-risk
What
Adds a new self-contained
agent/harness/fileaccesspackage, wired like the siblingagent/harness/todoprovider.New(*Options)returns aProviderbacked byagent.NewContextProvider, and itsProvidehook injects file tools plus instructions on each invocation.The provider exposes the same six tools as the .NET
FileAccessProvider:file_access_read_filefile_access_save_filefile_access_list_filesfile_access_list_subdirectoriesfile_access_search_filesfile_access_delete_fileAll operations go through a local-filesystem store rooted at a caller-granted
Options.RootDir(not session state).Options.ReadOnlyomits the save/delete tools, matching the .NET read-only shipping mode.Why
The Go harness tree (
agent/harness/) hadagentmode,loop,todo,toolapproval, andtoolautocall, but no file-access counterpart, while the .NET SDK shipsFileAccessProviderwith exactly this tool set (file_access_read_file/save_file/list_files/list_subdirectories/search_files/delete_file) and a read-only mode. This closes that cross-SDK parity gap so Go agents can be granted a scoped shared folder.Safety
Every path is interpreted relative to the root. The store resolves paths with
filepath.Clean(filepath.Join(root, rel))and rejects anything that is absolute or escapes the root via a prefix check, so../outside.txtand absolute paths are refused before touching the filesystem.Tests
fileaccess_test.gois black-box (package fileaccess_test) and reuses the same harness style astodo_test.go(agenttest.CreateSession, driving tools through the exportedInvokingAPI). It covers: default tool set + instructions present;ReadOnlyomitting save/delete; save->read round trip;list_filesdirect-children-only vslist_subdirectories;search_filesregex across nested files; delete; and path-escape rejection (../outside.txt, absolute path, escaping write not creating a file).go build ./...,go vet ./agent/harness/fileaccess/..., andgo test ./agent/harness/fileaccess/...all pass.Open design questions
search_filesreturning slash-separated relative paths matched against file contents) are chosen for parity; happy to align field names/semantics exactly with the .NET tool schemas if they differ.toolapprovalcould be a follow-up.