Add filememory harness context provider with session-scoped file_memory_* tools - #644
Conversation
1f716d8 to
eefdbe0
Compare
This comment has been minimized.
This comment has been minimized.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
awmgmcpg
To allow these domains, add them to the
network.allowedlist in your workflow frontmatter:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
Generated by Go API Consistency Review Agent · 69.7 AIC · ⌖ 4.49 AIC · ⊞ 5.7K
| var _ AgentFileStore = (*memoryStore)(nil) | ||
|
|
||
| // WriteInput is the input for the file_memory_write tool. | ||
| type WriteInput struct { |
There was a problem hiding this comment.
Parity gap — WriteInput missing description field
Both the .NET FileMemoryProvider.WriteAsync and the Python file_memory_write function accept an optional description parameter. This description is written as a companion _description.md file and is surfaced by file_memory_ls entries so the agent can discover what a file contains without reading it.
The Go WriteInput struct has only Path and Content. The upstream shape (both .NET and Python) is equivalent to:
type WriteInput struct {
Path string `json:"path"`
Content string `json:"content"`
Description string `json:"description,omitempty"` // optional file summary for discovery
}Upstream references:
- .NET:
FileMemoryProvider.WriteAsync(string fileName, string content, string? description = null, ...)indotnet/src/Microsoft.Agents.AI/Harness/FileMemory/FileMemoryProvider.cs - Python:
file_memory_write(file_name, content, description='', ...)inpython/packages/core/agent_framework/_harness/_file_memory.py
|
|
||
| lsTool := functool.MustNew( | ||
| functool.Config{ | ||
| Name: "file_memory_ls", |
There was a problem hiding this comment.
Parity gap — file_memory_ls returns []string instead of entries with descriptions
Upstream file_memory_ls in both .NET and Python returns a list of file entries that include the file name and its description (when one was written). The description field is what makes discovery practical for large sessions — the agent can read the index without opening every file.
The Go tool returns only paths ([]string). To align with upstream, file_memory_ls should return objects that carry at least {path, description}, and the AgentFileStore interface should expose a ListEntries() method that surfaces stored descriptions.
Upstream references:
- .NET:
FileListEntrytype andfile_memory_lsreturningIReadOnlyList<FileListEntry>indotnet/src/Microsoft.Agents.AI/Harness/FileMemory/FileListEntry.cs - Python:
file_memory_lsreturninglist[dict]withnameanddescriptionkeys inpython/packages/core/agent_framework/_harness/_file_memory.py
| }, | ||
| ) | ||
|
|
||
| return []tool.FuncTool{writeTool, readTool, deleteTool, lsTool, grepTool} |
There was a problem hiding this comment.
Parity gap — file_memory_replace and file_memory_replace_lines tools are missing
Both the .NET and Python implementations expose two additional editing tools that are absent from this Go port:
file_memory_replace— replaces occurrences of a substring within a named file in place, avoiding a full overwrite read-modify-write at the call sitefile_memory_replace_lines— replaces whole-line ranges within a file, enabling targeted edits to large stored documents
These tools are listed in the upstream AIContextProvider docs and instruction text as first-class capabilities. Agents prompted to use file_memory_replace or file_memory_replace_lines (e.g., when sharing prompts or examples across SDKs) will fail silently in Go because those tool names are simply not registered.
The AgentFileStore interface and the createTools helper should each be extended by two entries to close the gap.
Upstream references:
- .NET:
ReplaceToolName = "file_memory_replace"andReplaceLinesToolName = "file_memory_replace_lines"indotnet/src/Microsoft.Agents.AI/Harness/FileMemory/FileMemoryProvider.cs - Python:
file_memory_replaceandfile_memory_replace_linesasync functions inpython/packages/core/agent_framework/_harness/_file_memory.py
eefdbe0 to
ede4adb
Compare
…ry_* tools Add an agent/harness/filememory package that mirrors the todo harness provider wiring. It exposes file_memory_write, file_memory_read, file_memory_delete, file_memory_ls, and file_memory_grep tools backed by a self-contained in-memory AgentFileStore persisted through the session state bag, so files survive across invocations of the same session. This ports the .NET Harness FileMemoryProvider to Go for cross-SDK parity.
ede4adb to
6808c77
Compare
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Adds a new harness context provider (agent/harness/filememory) that exposes session-scoped “virtual file” tools backed by in-memory state persisted in agent.Session, aligning Go’s harness surface with the existing .NET FileMemory provider concept.
Changes:
- Introduces
filememory.Providerwith five tools:file_memory_write/read/delete/ls/grep, with per-session locking and session-state persistence. - Implements an in-memory
AgentFileStore(memoryStore) that is serialized into the session state bag across invocations. - Adds black-box tests validating tool availability, store behavior (write/read/ls/grep/delete), persistence across invocations, invalid regexp handling, and concurrent access under
-race.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| agent/harness/filememory/filememory.go | New FileMemory context provider + in-memory store + per-session locking + tool wiring. |
| agent/harness/filememory/filememory_test.go | New tests covering tool exposure, functional behavior, persistence, and concurrency safety. |
Suppressed comments (1)
agent/harness/filememory/filememory_test.go:225
- Same as above: prefer an exact comparison to "false" rather than substring matching for the delete-missing case to avoid false positives.
// Deleting a missing file reports false.
missing := callTool(t, outOpts, "file_memory_delete", `{"Arg0":"gone.txt"}`)
if !strings.Contains(missing, "false") {
t.Errorf("expected delete of missing file to report false, got %q", missing)
}
| readTool := functool.MustNew( | ||
| functool.Config{ | ||
| Name: "file_memory_read", | ||
| Description: "Read the content of a file previously written to session memory. Returns an empty string if the file does not exist.", | ||
| }, | ||
| func(ctx context.Context, path string) (string, error) { | ||
| mu := p.getSessionLock(opts) | ||
| mu.Lock() | ||
| defer mu.Unlock() | ||
| content, _ := p.loadStore(opts).Read(path) | ||
| return content, nil |
| removed := callTool(t, outOpts, "file_memory_delete", `{"Arg0":"gone.txt"}`) | ||
| if !strings.Contains(removed, "true") { | ||
| t.Errorf("expected delete to report true, got %q", removed) | ||
| } |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Generated by Go API Consistency Review Agent · sonnet46 · 41.3 AIC · ⌖ 4.99 AIC · ⊞ 5.7K
| ) | ||
|
|
||
| return []tool.FuncTool{writeTool, readTool, deleteTool, lsTool, grepTool} | ||
| } |
There was a problem hiding this comment.
Parity gap: missing file_memory_replace and file_memory_replace_lines tools
Both the .NET (FileMemoryProvider.cs, ReplaceToolName / ReplaceLinesToolName) and Python (_file_memory.py, file_memory_replace / file_memory_replace_lines) implementations expose two additional tools that this Go port does not include:
file_memory_replace— replaces occurrences of a substring within a file without rewriting the whole content.file_memory_replace_lines— replaces individual lines by number/pattern.
These tools are important for the typical agent workflow (incremental edits to stored plans/notes). Omitting them means agents using the Go SDK cannot perform in-place edits that .NET and Python agents can, leading to divergent agent behaviour.
Upstream references:
- .NET:
dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/FileMemoryProvider.cs—public const string ReplaceToolNameandpublic const string ReplaceLinesToolName - Python:
python/packages/core/agent_framework/_harness/_file_memory.py—file_memory_replaceandfile_memory_replace_linestools
Suggested action: Add both tools before merging, or open a follow-up issue tracking the gap so it is not forgotten.
| var _ AgentFileStore = (*memoryStore)(nil) | ||
|
|
||
| // WriteInput is the input for the file_memory_write tool. | ||
| type WriteInput struct { |
There was a problem hiding this comment.
Parity gap: file_memory_write missing optional description parameter
Both upstream implementations accept an optional description string on write, which is stored in a companion <filename>_description.md file and surfaced by file_memory_ls:
- .NET:
WriteAsync(string fileName, string content, string? description = null, ...) - Python:
file_memory_write(file_name: str, content: str, description: str | None = None) -> str
Go's WriteInput only has Path and Content, so the description feature is absent. This makes file_memory_ls less useful for agents managing many files (no human-readable summaries) and prevents Go agents from producing the same discovery-oriented metadata that .NET and Python agents produce.
Upstream references:
- .NET:
dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/FileMemoryProvider.cs—WriteAsyncsignature - Python:
python/packages/core/agent_framework/_harness/_file_memory.py—_WriteFileInputschema
Suggested action: Add an optional Description string field to WriteInput and persist a companion <path>_description.md file, or open a follow-up issue.
| scratch data, and intermediate results while working on a task. Files written | ||
| here persist across turns of the current session. | ||
|
|
||
| Use these tools to manage your files: |
There was a problem hiding this comment.
Parity gap: default instructions missing proactive index-check guidance
Upstream providers maintain a memories.md index that is injected into the agent context on each turn so the agent knows what files exist without calling file_memory_ls explicitly. The default instructions in both upstream implementations also explicitly tell the agent to check existing memories before starting new tasks:
"Before starting new tasks, use file_memory_ls and file_memory_grep to check for relevant existing memories to avoid duplicate work."
The Go default instructions omit both the proactive-check guidance and the index injection, so Go agents are more likely to duplicate work or fail to build on prior session state.
Upstream references:
- .NET:
dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/FileMemoryProvider.cs—DefaultInstructionsconstant andProvideAIContextAsync(index injection) - Python:
python/packages/core/agent_framework/_harness/_file_memory.py—DEFAULT_FILE_MEMORY_INSTRUCTIONSandbefore_run(index injection)
Suggested action: (a) Update defaultInstructions to include the proactive-check guidance, and (b) inject a memories.md-style summary listing current files into the agent context messages in provide().
|
PratikDhanave (@PratikDhanave) can you resolve the parity gaps? |
github-actions
Bot
added
area:agent
Cross-repo Parity Review —
|
| # | Issue | Upstream source | Inline comment |
|---|---|---|---|
| 1 | file_memory_replace and file_memory_replace_lines tools are missing |
Both .NET and Python | Line 8 |
| 2 | file_memory_write lacks the optional description parameter and sidecar _description.md logic |
Both .NET and Python | Line 120 |
| 3 | memories.md index is never maintained or injected into agent context |
Both .NET and Python | Line 229 |
Issue 1 is the highest-impact gap: the two missing tools are named explicitly in the upstream default instructions, so agents following the standard system prompt will attempt to call file_memory_replace and receive a tool-not-found error.
Issue 2 is closely related: the description field is what populates file_memory_ls summaries and the memories.md index (Issue 3). Addressing 2 and 3 together restores the full discovery loop that upstream agents rely on.
What is aligned ✅
file_memory_write,file_memory_read,file_memory_delete,file_memory_ls,file_memory_greptool names and semantics match upstream exactly.- Session-state persistence (
loadStore/saveStorevia the session state bag) is semantically equivalent to the .NETProvideAIContextAsyncstate loading. - Per-session mutex via weak-pointer identity is an idiomatic Go equivalent of the concurrent serialisation upstream uses.
- The exported
AgentFileStoreinterface and pluggable-backend design match the upstream abstraction (though Go's store is currently always in-memory, which is an explicitly acknowledged open design question in the PR description). public-api-changelabel is correctly present since this PR adds a new exported package and public types.
parity-approvedcannot be added until the three issues above are resolved or explicitly accepted as intentional divergence with a documented rationale.
Generated by Go API Consistency Review Agent · sonnet46 · 56.8 AIC · ⌖ 4.82 AIC · ⊞ 6K · ◷
There was a problem hiding this comment.
Generated by Go API Consistency Review Agent · sonnet46 · 56.8 AIC · ⌖ 4.82 AIC · ⊞ 6K
| // intermediate results across turns of a long-running task. | ||
| // | ||
| // The provider exposes five tools to the agent: file_memory_write, | ||
| // file_memory_read, file_memory_delete, file_memory_ls, and |
There was a problem hiding this comment.
Parity gap: missing file_memory_replace and file_memory_replace_lines tools
Both the .NET FileMemoryProvider and the Python FileMemoryProvider expose seven tools; this PR ships only five:
| Tool | .NET | Python | Go (this PR) |
|---|---|---|---|
file_memory_write |
✅ | ✅ | ✅ |
file_memory_read |
✅ | ✅ | ✅ |
file_memory_delete |
✅ | ✅ | ✅ |
file_memory_ls |
✅ | ✅ | ✅ |
file_memory_grep |
✅ | ✅ | ✅ |
file_memory_replace |
✅ | ✅ | ❌ missing |
file_memory_replace_lines |
✅ | ✅ | ❌ missing |
Upstream references:
- .NET:
FileMemoryProvider.cs— constantsReplaceToolName = "file_memory_replace"andReplaceLinesToolName = "file_memory_replace_lines" - Python:
_file_memory.py—file_memory_replaceandfile_memory_replace_linestool definitions
These two tools are not cosmetic additions. They are referenced by name in the default instructions of both upstream SDKs ("use file_memory_replace and file_memory_replace_lines to make small edits"), so agents that follow the standard system prompt will attempt to call them. Omitting them means the Go agent diverges at the behavioral contract level, not just the API surface level.
| var _ AgentFileStore = (*memoryStore)(nil) | ||
|
|
||
| // WriteInput is the input for the file_memory_write tool. | ||
| type WriteInput struct { |
There was a problem hiding this comment.
Parity gap: file_memory_write missing optional description parameter
Both upstream implementations accept an optional description argument on file_memory_write and use it to:
- Store a companion
*_description.mdsidecar file alongside the main content file. - Surface that description in
file_memory_lsoutput and the auto-maintainedmemories.mdindex, allowing the agent to discover file contents without reading them in full.
The Go WriteInput struct only carries Path and Content; it has no description field:
type WriteInput struct {
Path string `json:"path"`
Content string `json:"content"`
}Upstream .NET signature (from FileMemoryProvider.cs):
[Description("Write a memory file ... Include a description for large files ...")]
private async Task<string> WriteAsync(string fileName, string content, string? description = null, ...)Upstream Python WriteInput (from _file_memory.py):
description: Annotated[str | None, Field(description="Optional summary used to aid future discovery ...")]The description field enables the memories.md index mechanism (see next comment). Omitting it means files written by a Go agent carry no description metadata, and file_memory_ls cannot surface per-file summaries.
| func (p *Provider) provide(ctx context.Context, invoking agent.InvokingContext) ([]*message.Message, []agent.Option, error) { | ||
| opts := invoking.Options | ||
| tools := p.createTools(opts) | ||
|
|
There was a problem hiding this comment.
Parity gap: memories.md index is not maintained
Both .NET and Python automatically maintain a memories.md index file that is injected into the agent context on every Invoking call. The index lists each stored file with its description, letting the agent know what memories are available without exhausting its context window reading every file.
.NET (FileMemoryProvider.cs):
string indexPath = CombinePaths(state.WorkingFolder, MemoryIndexFileName);
string? indexContent = await this._fileStore.ReadAsync(indexPath, ...);
if (!string.IsNullOrWhiteSpace(indexContent))
// injected into AIContext as a system messagePython (_file_memory.py):
_MEMORY_INDEX_FILE_NAME = "memories.md"
# rebuilt on every write/delete and injected as a system prompt additionThe Go provide function injects only the static instruction string and the tools; it never reads back or injects a memories.md context message. This means a Go agent has no passive visibility into what files exist at the start of a turn — it must always call file_memory_ls first, whereas .NET/Python agents receive that summary for free in their context.
| const stateKey = "fileMemoryProviderState" | ||
|
|
||
| const defaultInstructions = `## File Memory | ||
|
|
There was a problem hiding this comment.
Parity gap: default instructions omit file_memory_replace / file_memory_replace_lines guidance
Because the two replace tools are missing, the Go default instructions also omit the guidance both upstream SDKs include:
.NET / Python default instructions both say:
Keep memories up-to-date by overwriting files when information changes, or by using
file_memory_replaceandfile_memory_replace_linesto make small edits.
The Go instructions only list five tools and say nothing about in-place editing. If the replace tools are added (see comment on line 8), the instructions should be updated accordingly.
github-actions
Bot
added
failed-auto-risk
What
Adds a new
agent/harness/filememorypackage: a context provider that gives agents a small session-scoped virtual file store for persisting notes, scratch data, and intermediate results across turns of a long-running task.The provider exposes five tools, constructed with
functool.MustNewand returned fromProvideasagent.WithTooloptions:file_memory_write— create or overwrite a filefile_memory_read— read a file's content (empty string when absent)file_memory_delete— remove a file (reports whether it existed)file_memory_ls— list stored file pathsfile_memory_grep— find files whose content matches a regexpState is held in a self-contained in-memory
AgentFileStoreand persisted through the session state bag (loadStore/saveStorekeyed by astateKey), so files survive acrossInvokingcalls of the same session. A per-session mutex (weak-pointer identity keyed, with runtime cleanup) serializes concurrent tool invocations, matching the todo provider.Why
This ports the .NET
Harness/FileMemory/FileMemoryProviderto Go. Go previously had no local file-memory harness — the only memory provider (foundryprovider/memory.go) is remote Foundry-store injection with no file tools and no local backend. The wiring mirrors the existingagent/harness/todoprovider exactly (agent.NewContextProvider+ProvidereturningWithTooloptions + session-state persistence), keeping the harness family consistent and aligning the Go SDK with the .NET surface.How it's tested
Black-box tests in
filememory_test.go(packagefilememory_test), reusing the same harness style astodo_test.go(agenttest.CreateSession, driving tools viatool.FuncTool.Call):Invokingreturns the fivefile_memory_*tools asWithTooloptions plus non-empty instructionsInvokingvia the session state bag-racego build ./...,go vet ./agent/harness/filememory/..., andgo test -race ./agent/harness/filememory/...all pass.Open design questions
AgentFileStoreinstead live in a shared package so multiple providers can reuse it?AgentFileStore(constructor option to inject a custom backend) desirable now, or should that wait for a concrete second backend (e.g. disk-backed)? Currently only the default in-memory store is wired.