From 423bf24ac02140e730ec10a385c78f837f52b68f Mon Sep 17 00:00:00 2001 From: Anton Sokolovskyi Date: Fri, 14 Aug 2026 22:03:46 +0200 Subject: [PATCH 1/2] .NET: [BREAKING] Add file_access_read_lines and align grep with the line editor The harness file tools were line-precise when editing (file_access_replace_lines takes 1-based line numbers and file_access_grep reports them) but all-or-nothing when reading, so there was no way to see the lines around a match without reading the whole file. Adds file_access_read_lines, rendering each row as `\t` with everything after the tab verbatim, including the line's own terminator, so a row feeds straight back into file_access_replace_lines. That contract only holds if grep and the line editor agree on what a line is, and they did not. The stores split on '\n' and stripped '\r'; FileEditor split on '\n', '\r\n' and a lone '\r' and kept terminators. So on a file using lone '\r' terminators, grep's line 1 addressed only part of what the model was shown and editing by that number silently changed the wrong text, and on a newline-terminated file grep could report a trailing line number the editor rejected as out of range. Both stores now use FileEditor.SplitLinesKeepEnds and report the matching line verbatim. BREAKING: FileSearchMatch.Line now includes the line's terminator, and line numbers change on content containing a lone '\r' or a trailing newline. This affects file_access_grep and file_memory_grep. The whole surface is [Experimental("MAAI001")]. Co-Authored-By: Claude Opus 5 (1M context) --- .../Claw_Step02_WorkingWithData/README.md | 3 +- .../Harness_Step03_DataProcessing/README.md | 3 +- .../Harness/FileAccess/FileAccessProvider.cs | 59 +++++- .../FileAccess/FileAccessProviderOptions.cs | 7 +- .../Harness/FileStore/FileEditor.cs | 55 +++++- .../Harness/FileStore/FileSearchMatch.cs | 7 +- .../FileStore/FileSystemAgentFileStore.cs | 14 +- .../FileStore/InMemoryAgentFileStore.cs | 14 +- .../HarnessAgentTests.cs | 1 + .../FileAccess/FileAccessProviderTests.cs | 181 +++++++++++++++++- .../Harness/FileStore/FileEditorTests.cs | 90 +++++++++ .../FileStore/InMemoryAgentFileStoreTests.cs | 72 ++++++- 12 files changed, 475 insertions(+), 31 deletions(-) diff --git a/dotnet/samples/02-agents/Harness/BuildYourOwnClaw/Claw_Step02_WorkingWithData/README.md b/dotnet/samples/02-agents/Harness/BuildYourOwnClaw/Claw_Step02_WorkingWithData/README.md index 9755e8ba6b..892d346c4f 100644 --- a/dotnet/samples/02-agents/Harness/BuildYourOwnClaw/Claw_Step02_WorkingWithData/README.md +++ b/dotnet/samples/02-agents/Harness/BuildYourOwnClaw/Claw_Step02_WorkingWithData/README.md @@ -17,7 +17,8 @@ It builds on Post 1's personal finance assistant and teaches it to work with *yo > ⚠️ **Security — avoid tool-name collisions:** auto-approval rules such as > `FileAccessProvider.ReadOnlyToolsAutoApprovalRule` match tool calls **solely by tool name**. Any - > other registered tool that shares one of the approved names (`file_access_read`, `file_access_ls`, + > other registered tool that shares one of the approved names (`file_access_read`, + > `file_access_read_lines`, `file_access_ls`, > `file_access_grep`) would be silently auto-approved, bypassing the human > approval boundary. Ensure no other tool's name collides with the reserved names a rule approves. - **Durable memory, two ways:** diff --git a/dotnet/samples/02-agents/Harness/Harness_Step03_DataProcessing/README.md b/dotnet/samples/02-agents/Harness/Harness_Step03_DataProcessing/README.md index 02b15847e9..2415efdb3b 100644 --- a/dotnet/samples/02-agents/Harness/Harness_Step03_DataProcessing/README.md +++ b/dotnet/samples/02-agents/Harness/Harness_Step03_DataProcessing/README.md @@ -55,7 +55,8 @@ E.g. try the following prompt `Please process the sales.csv file by first filter This sample uses `FileAccessProvider.ReadOnlyToolsAutoApprovalRule` to auto-approve read-only file access tools. Built-in auto-approval rules match tool calls **solely by tool name**, so any other -registered tool that shares one of the approved names (`file_access_read`, `file_access_ls`, +registered tool that shares one of the approved names (`file_access_read`, `file_access_read_lines`, +`file_access_ls`, `file_access_grep`) would be **silently auto-approved**, bypassing the human approval boundary. Ensure no other tool's name collides with the reserved names an auto-approval rule approves. diff --git a/dotnet/src/Microsoft.Agents.AI/Harness/FileAccess/FileAccessProvider.cs b/dotnet/src/Microsoft.Agents.AI/Harness/FileAccess/FileAccessProvider.cs index f42335b798..72ad41156a 100644 --- a/dotnet/src/Microsoft.Agents.AI/Harness/FileAccess/FileAccessProvider.cs +++ b/dotnet/src/Microsoft.Agents.AI/Harness/FileAccess/FileAccessProvider.cs @@ -5,6 +5,7 @@ using System.ComponentModel; using System.Diagnostics.CodeAnalysis; using System.Linq; +using System.Text; using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.AI; @@ -37,6 +38,7 @@ namespace Microsoft.Agents.AI; /// /// file_access_write — Write a file with the given name and content. /// file_access_read — Read the content of a file by name. +/// file_access_read_lines — Read a range of lines from a file by line number. /// file_access_delete — Delete a file by name. /// file_access_ls — List the direct child files and subdirectories of a directory. /// file_access_grep — Recursively search file contents using a regular expression pattern. @@ -44,12 +46,13 @@ namespace Microsoft.Agents.AI; /// file_access_replace_lines — Replace whole lines within a file. /// /// When is set, only the read-only tools -/// (file_access_read, file_access_ls, and file_access_grep) are exposed. +/// (file_access_read, file_access_read_lines, file_access_ls, and +/// file_access_grep) are exposed. /// /// /// By default, all of these tools require approval: each is exposed as an . /// Approval can be disabled per group via -/// (read, ls, and grep) and +/// (read, read_lines, ls, and grep) and /// (write, delete, replace, and replace_lines). /// /// @@ -57,8 +60,8 @@ namespace Microsoft.Agents.AI; /// : /// /// -/// — auto-approves only the read-only tools (read, ls, -/// and grep), while still prompting for the tools that modify the store (write, delete, replace, and replace_lines). +/// — auto-approves only the read-only tools (read, read_lines, +/// ls, and grep), while still prompting for the tools that modify the store (write, delete, replace, and replace_lines). /// /// /// — auto-approves every file access tool, including the tools that modify the store. @@ -82,6 +85,9 @@ public sealed class FileAccessProvider : AIContextProvider, IDisposable /// The name of the tool that reads a file. public const string ReadFileToolName = "file_access_read"; + /// The name of the tool that reads a range of lines from a file. + public const string ReadLinesToolName = "file_access_read_lines"; + /// The name of the tool that deletes a file. public const string DeleteFileToolName = "file_access_delete"; @@ -101,6 +107,7 @@ public sealed class FileAccessProvider : AIContextProvider, IDisposable private static readonly HashSet s_readOnlyToolNames = new(StringComparer.Ordinal) { ReadFileToolName, + ReadLinesToolName, LsToolName, GrepToolName, }; @@ -110,6 +117,7 @@ public sealed class FileAccessProvider : AIContextProvider, IDisposable { WriteToolName, ReadFileToolName, + ReadLinesToolName, DeleteFileToolName, LsToolName, GrepToolName, @@ -129,6 +137,9 @@ These files persist beyond the current session and may be shared across sessions or `file_access_grep` to search file contents recursively across the whole store. - To make small edits to an existing file, prefer `file_access_replace` (substring replacement) or `file_access_replace_lines` (whole-line replacement) over rewriting the whole file. + - To change part of a file, find the line numbers with `file_access_grep`, read the range around them + with `file_access_read_lines`, then edit with `file_access_replace_lines`. Reading the whole file + first is rarely necessary. """; private readonly AgentFileStore _fileStore; @@ -161,7 +172,8 @@ public FileAccessProvider(AgentFileStore fileStore, FileAccessProviderOptions? o /// /// Gets an auto-approval rule that approves the read-only file access tools - /// (, , and ). + /// (, , , + /// and ). /// /// /// @@ -179,6 +191,7 @@ public FileAccessProvider(AgentFileStore fileStore, FileAccessProviderOptions? o /// This rule approves calls to exactly the following tool names: /// /// (file_access_read) + /// (file_access_read_lines) /// (file_access_ls) /// (file_access_grep) /// @@ -213,6 +226,7 @@ public FileAccessProvider(AgentFileStore fileStore, FileAccessProviderOptions? o /// /// (file_access_write) /// (file_access_read) + /// (file_access_read_lines) /// (file_access_delete) /// (file_access_ls) /// (file_access_grep) @@ -296,6 +310,40 @@ private async Task ReadAsync(string fileName, CancellationToken cancella return content ?? $"File '{fileName}' not found."; } + /// + /// Read a range of lines from a file, each prefixed with its 1-based line number and a tab. + /// + /// The name of the file to read. + /// The 1-based line number to read from. + /// The 1-based line number to read through, inclusive. When , reads to the end of the file. + /// A token to cancel the operation. + /// The numbered lines, or a not-found message. + /// + /// Thrown when either bound is not positive, when precedes + /// , or when is past the last line. + /// + [Description("Read part of a file by 1-based inclusive line number; omit end_line to read to the end of the file, and an end_line past the last line is clamped. Line numbers match file_access_grep and file_access_replace_lines. Each line is prefixed with its number and a tab; everything after that tab is verbatim, including the line's own terminator, so it can be reused as a file_access_replace_lines new_line.")] + private async Task ReadLinesAsync(string fileName, int startLine, int? endLine = null, CancellationToken cancellationToken = default) + { + string path = StorePaths.NormalizeRelativePath(fileName); + string? content = await this._fileStore.ReadAsync(path, cancellationToken).ConfigureAwait(false); + if (content is null) + { + return $"File '{fileName}' not found."; + } + + List lines = FileEditor.SliceLines(content, startLine, endLine); + + // Each line keeps its terminator, so it doubles as the row separator. + var builder = new StringBuilder(); + for (int i = 0; i < lines.Count; i++) + { + builder.Append(startLine + i).Append('\t').Append(lines[i]); + } + + return builder.ToString(); + } + /// /// Delete a file by name. /// @@ -460,6 +508,7 @@ private AITool[] CreateTools() var tools = new List { WrapWithApprovalIfRequired(AIFunctionFactory.Create(this.ReadAsync, new AIFunctionFactoryOptions { Name = ReadFileToolName, SerializerOptions = serializerOptions }), readOnlyRequiresApproval), + WrapWithApprovalIfRequired(AIFunctionFactory.Create(this.ReadLinesAsync, new AIFunctionFactoryOptions { Name = ReadLinesToolName, SerializerOptions = serializerOptions }), readOnlyRequiresApproval), WrapWithApprovalIfRequired(AIFunctionFactory.Create(this.LsAsync, new AIFunctionFactoryOptions { Name = LsToolName, SerializerOptions = serializerOptions }), readOnlyRequiresApproval), WrapWithApprovalIfRequired(AIFunctionFactory.Create(this.GrepAsync, new AIFunctionFactoryOptions { Name = GrepToolName, SerializerOptions = serializerOptions }), readOnlyRequiresApproval), }; diff --git a/dotnet/src/Microsoft.Agents.AI/Harness/FileAccess/FileAccessProviderOptions.cs b/dotnet/src/Microsoft.Agents.AI/Harness/FileAccess/FileAccessProviderOptions.cs index 8f8e406e48..c26b4781ce 100644 --- a/dotnet/src/Microsoft.Agents.AI/Harness/FileAccess/FileAccessProviderOptions.cs +++ b/dotnet/src/Microsoft.Agents.AI/Harness/FileAccess/FileAccessProviderOptions.cs @@ -25,7 +25,8 @@ public sealed class FileAccessProviderOptions /// /// /// When (the default), all tools are exposed. When , - /// only the read-only tools (file_access_read, file_access_ls, and file_access_grep) + /// only the read-only tools (file_access_read, file_access_read_lines, file_access_ls, + /// and file_access_grep) /// are exposed; the tools that modify the store (file_access_write, file_access_delete, /// file_access_replace, and file_access_replace_lines) are hidden. /// @@ -33,8 +34,8 @@ public sealed class FileAccessProviderOptions /// /// Gets or sets a value indicating whether approval is disabled for the read-only file access tools - /// (, , - /// and ). + /// (, , + /// , and ). /// /// /// When (the default), these tools require approval before invocation. diff --git a/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileEditor.cs b/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileEditor.cs index 32c3ff8478..a8fca086b9 100644 --- a/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileEditor.cs +++ b/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileEditor.cs @@ -7,7 +7,8 @@ namespace Microsoft.Agents.AI; /// /// Internal helpers shared by and -/// for the replace and replace_lines tools. +/// for the replace, replace_lines, and read_lines tools, and by the file stores +/// for grep. /// internal static class FileEditor { @@ -92,6 +93,52 @@ internal static string ApplyReplaceLines(string content, IReadOnlyList + /// Returns the 1-based inclusive [startLine, endLine] slice of , + /// with each line's terminator kept attached. An past the last line is + /// clamped, and omitting it reads to the end of the content. + /// + /// + /// Thrown when either bound is not positive, when precedes + /// , or when is past the last line. + /// + internal static List SliceLines(string content, int startLine, int? endLine) + { + List lines = SplitLinesKeepEnds(content); + int total = lines.Count; + + if (startLine < 1) + { + throw new ArgumentException($"start_line must be a positive integer, got {startLine}."); + } + + if (endLine is < 1) + { + throw new ArgumentException($"end_line must be a positive integer, got {endLine}."); + } + + if (endLine < startLine) + { + throw new ArgumentException($"end_line ({endLine}) must not be less than start_line ({startLine})."); + } + + if (startLine > total) + { + throw new ArgumentException($"start_line {startLine} is out of range (file has {total} lines)."); + } + + // Clamping end_line rather than failing keeps "read from here to the end" a single call. + int lastLine = endLine is null ? total : Math.Min(endLine.Value, total); + return lines.GetRange(startLine - 1, lastLine - startLine + 1); + } + + /// + /// Returns without the trailing \n that terminates it, so search + /// patterns are matched against a line's text rather than its line break. + /// + internal static string TrimTrailingNewline(string line) + => line.EndsWith("\n", StringComparison.Ordinal) ? line.Substring(0, line.Length - 1) : line; + private static int CountOccurrences(string content, string value) { int count = 0; @@ -109,7 +156,11 @@ private static int CountOccurrences(string content, string value) /// Splits content into lines, keeping each line's trailing newline (\r\n, \n, or a lone /// \r) attached. The final line has no terminator when the content does not end with a newline. /// - private static List SplitLinesKeepEnds(string content) + /// + /// This is the single definition of a "line" shared by the search and line-edit tools, so the line + /// numbers reported by grep address the same lines that replace_lines edits. + /// + internal static List SplitLinesKeepEnds(string content) { var lines = new List(); int start = 0; diff --git a/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileSearchMatch.cs b/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileSearchMatch.cs index 0bf2d102d3..d7427a788b 100644 --- a/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileSearchMatch.cs +++ b/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileSearchMatch.cs @@ -19,8 +19,13 @@ public sealed class FileSearchMatch public int LineNumber { get; set; } /// - /// Gets or sets the content of the matching line. + /// Gets or sets the matching line, verbatim. /// + /// + /// The line keeps its own terminator (\r\n, \n, or a lone \r), except on a final + /// line that the content does not terminate. Together with addressing the + /// same lines the line-edit tools use, this makes the value reusable as a literal replacement line. + /// [JsonPropertyName("line")] public string Line { get; set; } = string.Empty; } diff --git a/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileSystemAgentFileStore.cs b/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileSystemAgentFileStore.cs index 8f3d171c94..12ba946c14 100644 --- a/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileSystemAgentFileStore.cs +++ b/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileSystemAgentFileStore.cs @@ -204,17 +204,19 @@ public override async Task> SearchAsync( #endif // Search each line for regex matches, tracking line numbers and building a snippet. - string[] lines = fileContent.Split('\n'); + // Lines keep their terminators, so these line numbers address the same lines that + // replace_lines edits and each reported line can be reused as a literal new_line. + List lines = FileEditor.SplitLinesKeepEnds(fileContent); var matchingLines = new List(); string? firstSnippet = null; int lineStartOffset = 0; - for (int i = 0; i < lines.Length; i++) + for (int i = 0; i < lines.Count; i++) { - Match match = regex.Match(lines[i]); + Match match = regex.Match(FileEditor.TrimTrailingNewline(lines[i])); if (match.Success) { - matchingLines.Add(new FileSearchMatch { LineNumber = i + 1, Line = lines[i].TrimEnd('\r') }); + matchingLines.Add(new FileSearchMatch { LineNumber = i + 1, Line = lines[i] }); // Build a context snippet around the first match (±50 chars). if (firstSnippet is null) @@ -226,8 +228,8 @@ public override async Task> SearchAsync( } } - // Advance the offset past this line (including the '\n' separator). - lineStartOffset += lines[i].Length + 1; + // Advance the offset past this line; its terminator is already part of its length. + lineStartOffset += lines[i].Length; } if (matchingLines.Count > 0) diff --git a/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/InMemoryAgentFileStore.cs b/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/InMemoryAgentFileStore.cs index 62dfc020cb..bafd604837 100644 --- a/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/InMemoryAgentFileStore.cs +++ b/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/InMemoryAgentFileStore.cs @@ -142,18 +142,20 @@ public override Task> SearchAsync(string directo } // Search each line for regex matches, tracking line numbers and building a snippet. + // Lines keep their terminators, so these line numbers address the same lines that + // replace_lines edits and each reported line can be reused as a literal new_line. string fileContent = kvp.Value; - string[] lines = fileContent.Split('\n'); + List lines = FileEditor.SplitLinesKeepEnds(fileContent); var matchingLines = new List(); string? firstSnippet = null; int lineStartOffset = 0; - for (int i = 0; i < lines.Length; i++) + for (int i = 0; i < lines.Count; i++) { - Match match = regex.Match(lines[i]); + Match match = regex.Match(FileEditor.TrimTrailingNewline(lines[i])); if (match.Success) { - matchingLines.Add(new FileSearchMatch { LineNumber = i + 1, Line = lines[i].TrimEnd('\r') }); + matchingLines.Add(new FileSearchMatch { LineNumber = i + 1, Line = lines[i] }); // Build a context snippet around the first match (±50 chars). if (firstSnippet is null) @@ -165,8 +167,8 @@ public override Task> SearchAsync(string directo } } - // Advance the offset past this line (including the '\n' separator). - lineStartOffset += lines[i].Length + 1; + // Advance the offset past this line; its terminator is already part of its length. + lineStartOffset += lines[i].Length; } if (matchingLines.Count > 0) diff --git a/dotnet/tests/Microsoft.Agents.AI.Harness.UnitTests/HarnessAgentTests.cs b/dotnet/tests/Microsoft.Agents.AI.Harness.UnitTests/HarnessAgentTests.cs index b01af37e0f..feebac05f8 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Harness.UnitTests/HarnessAgentTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Harness.UnitTests/HarnessAgentTests.cs @@ -1386,6 +1386,7 @@ public async Task FileAccessProvider_UsesProvidedOptionsAsync() // DisableWriteTools = true => only the read-only tools are exposed. Assert.Contains(FileAccessProvider.ReadFileToolName, toolNames); + Assert.Contains(FileAccessProvider.ReadLinesToolName, toolNames); Assert.Contains(FileAccessProvider.LsToolName, toolNames); Assert.Contains(FileAccessProvider.GrepToolName, toolNames); Assert.DoesNotContain(FileAccessProvider.WriteToolName, toolNames); diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileAccess/FileAccessProviderTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileAccess/FileAccessProviderTests.cs index e8797a4bc9..df66a11852 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileAccess/FileAccessProviderTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileAccess/FileAccessProviderTests.cs @@ -43,8 +43,8 @@ public async Task ProvideAIContextAsync_ReturnsToolsAsync() // Arrange var tools = await CreateToolsAsync(); - // Assert — 7 tools: Read, Ls, Grep, Write, Delete, Replace, ReplaceLines - Assert.Equal(7, tools.Count()); + // Assert — 8 tools: Read, ReadLines, Ls, Grep, Write, Delete, Replace, ReplaceLines + Assert.Equal(8, tools.Count()); } #endregion @@ -58,7 +58,7 @@ public async Task ProvideAIContextAsync_AllToolsRequireApprovalAsync() var tools = await CreateToolsAsync(); // Assert — every tool is wrapped so that it always requires approval. - Assert.Equal(7, tools.Count()); + Assert.Equal(8, tools.Count()); Assert.All(tools, tool => Assert.IsType(tool)); } @@ -105,7 +105,7 @@ public async Task DisableBothToolApprovals_NoToolsWrappedAsync() })).ToList(); // Assert — no tool requires approval. - Assert.Equal(7, tools.Count); + Assert.Equal(8, tools.Count); Assert.DoesNotContain(tools, tool => tool is ApprovalRequiredAIFunction); } @@ -117,6 +117,7 @@ private static void AssertRequiresApproval(IEnumerable tools, string too [Theory] [InlineData(FileAccessProvider.ReadFileToolName, true)] + [InlineData(FileAccessProvider.ReadLinesToolName, true)] [InlineData(FileAccessProvider.LsToolName, true)] [InlineData(FileAccessProvider.GrepToolName, true)] [InlineData(FileAccessProvider.WriteToolName, false)] @@ -138,6 +139,7 @@ public async Task ReadOnlyToolsAutoApprovalRule_ApprovesOnlyReadOnlyToolsAsync(s [Theory] [InlineData(FileAccessProvider.ReadFileToolName, true)] + [InlineData(FileAccessProvider.ReadLinesToolName, true)] [InlineData(FileAccessProvider.LsToolName, true)] [InlineData(FileAccessProvider.GrepToolName, true)] [InlineData(FileAccessProvider.WriteToolName, true)] @@ -375,6 +377,174 @@ public async Task ReadFile_NonExistent_ReturnsNotFoundMessageAsync() #endregion + #region ReadLines Tests + + [Fact] + public async Task ReadLines_ReturnsNumberedInclusiveRangeAsync() + { + // Arrange + var store = new InMemoryAgentFileStore(); + await store.WriteAsync("notes.md", "one\ntwo\nthree\nfour\n"); + var tools = await CreateToolsAsync(store); + var readLines = GetTool(tools, "file_access_read_lines"); + + // Act + var result = await InvokeToolAsync(readLines, new AIFunctionArguments + { + ["fileName"] = "notes.md", + ["startLine"] = 2, + ["endLine"] = 3, + }); + + // Assert — each line keeps its terminator, which doubles as the row separator. + var text = Assert.IsType(result).GetString(); + Assert.Equal("2\ttwo\n3\tthree\n", text); + } + + [Fact] + public async Task ReadLines_OmittedEndLine_ReadsToEndOfFileAsync() + { + // Arrange + var store = new InMemoryAgentFileStore(); + await store.WriteAsync("notes.md", "one\ntwo\nthree"); + var tools = await CreateToolsAsync(store); + var readLines = GetTool(tools, "file_access_read_lines"); + + // Act + var result = await InvokeToolAsync(readLines, new AIFunctionArguments + { + ["fileName"] = "notes.md", + ["startLine"] = 2, + }); + + // Assert — the last line has no terminator, so the output ends without one. + var text = Assert.IsType(result).GetString(); + Assert.Equal("2\ttwo\n3\tthree", text); + } + + [Fact] + public async Task ReadLines_EndLinePastLastLine_IsClampedAsync() + { + // Arrange + var store = new InMemoryAgentFileStore(); + await store.WriteAsync("notes.md", "one\ntwo\n"); + var tools = await CreateToolsAsync(store); + var readLines = GetTool(tools, "file_access_read_lines"); + + // Act + var result = await InvokeToolAsync(readLines, new AIFunctionArguments + { + ["fileName"] = "notes.md", + ["startLine"] = 1, + ["endLine"] = 99, + }); + + // Assert — clamping, not an error. + var text = Assert.IsType(result).GetString(); + Assert.Equal("1\tone\n2\ttwo\n", text); + } + + [Fact] + public async Task ReadLines_PreservesCrlfTerminatorsAsync() + { + // Arrange + var store = new InMemoryAgentFileStore(); + await store.WriteAsync("notes.md", "alpha\r\nbeta\r\n"); + var tools = await CreateToolsAsync(store); + var readLines = GetTool(tools, "file_access_read_lines"); + + // Act + var result = await InvokeToolAsync(readLines, new AIFunctionArguments + { + ["fileName"] = "notes.md", + ["startLine"] = 1, + ["endLine"] = 1, + }); + + // Assert — the line's own terminator is reported, so no detection step is needed. + var text = Assert.IsType(result).GetString(); + Assert.Equal("1\talpha\r\n", text); + } + + [Fact] + public async Task ReadLines_NonExistent_ReturnsNotFoundMessageAsync() + { + // Arrange + var tools = await CreateToolsAsync(); + var readLines = GetTool(tools, "file_access_read_lines"); + + // Act + var result = await InvokeToolAsync(readLines, new AIFunctionArguments + { + ["fileName"] = "nonexistent.md", + ["startLine"] = 1, + }); + + // Assert — same shape as file_access_read. + var text = Assert.IsType(result).GetString(); + Assert.Contains("not found", text); + } + + [Fact] + public async Task ReadLines_StartLinePastLastLine_ThrowsAsync() + { + // Arrange + var store = new InMemoryAgentFileStore(); + await store.WriteAsync("notes.md", "one\ntwo\n"); + var tools = await CreateToolsAsync(store); + var readLines = GetTool(tools, "file_access_read_lines"); + + // Act & Assert — exception bubbles, as it does for replace_lines. + await Assert.ThrowsAsync(async () => + await InvokeToolAsync(readLines, new AIFunctionArguments + { + ["fileName"] = "notes.md", + ["startLine"] = 3, + })); + } + + [Fact] + public async Task ReadLines_RoundTripsAGrepMatchIntoReplaceLinesAsync() + { + // Arrange — a CRLF file with a trailing newline, the case where the terminator used to be lost. + var store = new InMemoryAgentFileStore(); + await store.WriteAsync("notes.md", "alpha\r\nbeta needle\r\ngamma\r\n"); + var tools = await CreateToolsAsync(store); + + // Act — grep for the line, read that line number back, then feed the result to replace_lines. + var grepResult = await InvokeToolAsync(GetTool(tools, "file_access_grep"), new AIFunctionArguments + { + ["regexPattern"] = "needle", + }); + JsonElement match = Assert.IsType(grepResult).EnumerateArray().Single() + .GetProperty("matchingLines").EnumerateArray().Single(); + int lineNumber = match.GetProperty("lineNumber").GetInt32(); + + var readResult = await InvokeToolAsync(GetTool(tools, "file_access_read_lines"), new AIFunctionArguments + { + ["fileName"] = "notes.md", + ["startLine"] = lineNumber, + ["endLine"] = lineNumber, + }); + string shown = Assert.IsType(readResult).GetString()!; + + // Everything after the number and tab is the line verbatim, so it is already a valid new_line. + string line = shown.Substring(shown.IndexOf('\t') + 1); + await InvokeToolAsync(GetTool(tools, "file_access_replace_lines"), new AIFunctionArguments + { + ["fileName"] = "notes.md", + ["edits"] = new List { new() { LineNumber = lineNumber, NewLine = line.ToUpperInvariant() } }, + }); + + // Assert — grep, read_lines and replace_lines agree on line 2, and the CRLF survives. + Assert.Equal(2, lineNumber); + Assert.Equal("beta needle\r\n", match.GetProperty("line").GetString()); + Assert.Equal("2\tbeta needle\r\n", shown); + Assert.Equal("alpha\r\nBETA NEEDLE\r\ngamma\r\n", await store.ReadAsync("notes.md")); + } + + #endregion + #region DeleteFile Tests [Fact] @@ -874,8 +1044,9 @@ public async Task Options_DisableWriteTools_OnlyExposesReadOnlyToolsAsync() var names = result.Tools!.OfType().Select(t => t.Name).ToList(); // Assert — only read-only tools are exposed. - Assert.Equal(3, names.Count); + Assert.Equal(4, names.Count); Assert.Contains(FileAccessProvider.ReadFileToolName, names); + Assert.Contains(FileAccessProvider.ReadLinesToolName, names); Assert.Contains(FileAccessProvider.LsToolName, names); Assert.Contains(FileAccessProvider.GrepToolName, names); Assert.DoesNotContain(FileAccessProvider.WriteToolName, names); diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileStore/FileEditorTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileStore/FileEditorTests.cs index 505e761ca9..0ed63dfdec 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileStore/FileEditorTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileStore/FileEditorTests.cs @@ -178,4 +178,94 @@ public void ApplyReplaceLines_EmbeddedNewLine_ExpandsIntoMultipleLines() } #endregion + + #region SplitLinesKeepEnds + + [Theory] + [InlineData("a\nb\nc", new[] { "a\n", "b\n", "c" })] + [InlineData("a\nb\n", new[] { "a\n", "b\n" })] + [InlineData("a\r\nb\r\n", new[] { "a\r\n", "b\r\n" })] + [InlineData("a\rb\rc", new[] { "a\r", "b\r", "c" })] + [InlineData("a\r\nb\nc\r", new[] { "a\r\n", "b\n", "c\r" })] + [InlineData("single", new[] { "single" })] + [InlineData("", new string[0])] + public void SplitLinesKeepEnds_KeepsEachLinesOwnTerminator(string content, string[] expected) + { + // Act + List lines = FileEditor.SplitLinesKeepEnds(content); + + // Assert + Assert.Equal(expected, lines); + } + + [Fact] + public void SplitLinesKeepEnds_ConcatenationRoundTripsTheContent() + { + // Arrange — mixed terminators, the case a whole-file read would otherwise be needed to detect. + const string Content = "alpha\r\nbeta\ngamma\rdelta"; + + // Act + List lines = FileEditor.SplitLinesKeepEnds(Content); + + // Assert — nothing is lost or added, which is what makes a reported line reusable verbatim. + Assert.Equal(Content, string.Concat(lines)); + } + + #endregion + + #region SliceLines + + [Fact] + public void SliceLines_ReturnsInclusiveRangeWithTerminators() + { + // Act + List lines = FileEditor.SliceLines("one\ntwo\nthree\nfour\n", 2, 3); + + // Assert + Assert.Equal(2, lines.Count); + Assert.Equal("two\nthree\n", string.Concat(lines)); + } + + [Fact] + public void SliceLines_NullEndLine_ReadsToEndOfContent() + { + // Act + List lines = FileEditor.SliceLines("one\ntwo\nthree", 2, endLine: null); + + // Assert + Assert.Equal(2, lines.Count); + Assert.Equal("two\nthree", string.Concat(lines)); + } + + [Fact] + public void SliceLines_EndLinePastLastLine_IsClamped() + { + // Act + List lines = FileEditor.SliceLines("one\ntwo\n", 1, 99); + + // Assert + Assert.Equal(2, lines.Count); + Assert.Equal("one\ntwo\n", string.Concat(lines)); + } + + [Theory] + [InlineData(0, null)] + [InlineData(-1, null)] + [InlineData(1, 0)] + [InlineData(3, 2)] + [InlineData(4, null)] + public void SliceLines_InvalidRange_Throws(int startLine, int? endLine) + { + // Act & Assert — "one\ntwo\nthree" has three lines. + Assert.Throws(() => FileEditor.SliceLines("one\ntwo\nthree", startLine, endLine)); + } + + [Fact] + public void SliceLines_EmptyContent_HasNoAddressableLines() + { + // Act & Assert — matches ApplyReplaceLines, which also rejects line 1 of an empty file. + Assert.Throws(() => FileEditor.SliceLines(string.Empty, 1, null)); + } + + #endregion } diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileStore/InMemoryAgentFileStoreTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileStore/InMemoryAgentFileStoreTests.cs index 722dc8f735..2d64c8401a 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileStore/InMemoryAgentFileStoreTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileStore/InMemoryAgentFileStoreTests.cs @@ -207,11 +207,81 @@ public async Task SearchFiles_ReturnsMatchingLineNumbersAsync() Assert.Single(results); Assert.Equal(2, results[0].MatchingLines.Count); Assert.Equal(2, results[0].MatchingLines[0].LineNumber); - Assert.Equal("Line two with match", results[0].MatchingLines[0].Line); + // Lines are reported verbatim, so an interior line keeps its terminator. + Assert.Equal("Line two with match\n", results[0].MatchingLines[0].Line); Assert.Equal(4, results[0].MatchingLines[1].LineNumber); + // The last line has no terminator in the content, so none is reported. Assert.Equal("Line four with match", results[0].MatchingLines[1].Line); } + [Fact] + public async Task SearchFiles_ReportsCrlfLinesVerbatimAsync() + { + // Arrange + var store = new InMemoryAgentFileStore(); + await store.WriteAsync("folder/notes.md", "alpha\r\nbeta match\r\ngamma\r\n"); + + // Act + var results = await store.SearchAsync("folder", "match"); + + // Assert — the CRLF is preserved, so the line can be fed back to replace_lines unchanged. + Assert.Single(results); + Assert.Single(results[0].MatchingLines); + Assert.Equal(2, results[0].MatchingLines[0].LineNumber); + Assert.Equal("beta match\r\n", results[0].MatchingLines[0].Line); + } + + [Fact] + public async Task SearchFiles_TrailingNewline_DoesNotReportAnExtraLineAsync() + { + // Arrange — a newline-terminated file has as many lines as the line editor sees, not one more. + var store = new InMemoryAgentFileStore(); + await store.WriteAsync("folder/notes.md", "a\nb\n"); + + // Act — a pattern that also matches an empty line. + var results = await store.SearchAsync("folder", "^.*$"); + + // Assert + Assert.Single(results); + Assert.Equal(2, results[0].MatchingLines.Count); + Assert.Equal("a\n", results[0].MatchingLines[0].Line); + Assert.Equal("b\n", results[0].MatchingLines[1].Line); + } + + [Fact] + public async Task SearchFiles_LoneCarriageReturn_SplitsLikeTheLineEditorAsync() + { + // Arrange — a lone '\r' terminates a line for the line editor, so grep must agree. + var store = new InMemoryAgentFileStore(); + await store.WriteAsync("folder/notes.md", "alpha\rbeta match\rgamma"); + + // Act + var results = await store.SearchAsync("folder", "match"); + + // Assert + Assert.Single(results); + Assert.Single(results[0].MatchingLines); + Assert.Equal(2, results[0].MatchingLines[0].LineNumber); + Assert.Equal("beta match\r", results[0].MatchingLines[0].Line); + } + + [Fact] + public async Task SearchFiles_SnippetIsAnchoredAtTheMatchAsync() + { + // Arrange — the leading line is long enough that the ±50 char snippet window is not clamped to + // the start of the file, so an off-by-one in the per-line offset would shift the snippet. + var store = new InMemoryAgentFileStore(); + string padding = new('x', 60); + await store.WriteAsync("folder/notes.md", $"{padding}\nneedle\n"); + + // Act + var results = await store.SearchAsync("folder", "needle"); + + // Assert — the match starts at index 61, so the snippet starts at index 11. + Assert.Single(results); + Assert.Equal($"{new string('x', 49)}\nneedle\n", results[0].Snippet); + } + [Fact] public async Task SearchFiles_CaseInsensitiveAsync() { From 31bc9939aa0277ca992ff3642d62ae858fdcdb02 Mon Sep 17 00:00:00 2001 From: Anton Sokolovskyi Date: Fri, 14 Aug 2026 22:31:26 +0200 Subject: [PATCH 2/2] .NET: Strip the whole line terminator before matching, and name args as the schema does Addresses both review comments on #7671. TrimTrailingNewline removed only "\n", so grep matched against text such as "match\r" on CRLF and lone-CR lines and an end-anchored pattern like "match$" failed even though the line's text was exactly "match". Renamed to TrimLineTerminator and it now strips "\r\n", "\n", or a lone "\r". The file_access_read_lines description and the SliceLines failure messages referred to end_line/start_line, but the generated schema exposes the arguments as endLine/startLine, so the model could be prompted to emit an invalid argument name. Both now use the schema's names. (new_line is left as-is: FileLineEdit sets it explicitly via JsonPropertyName.) Co-Authored-By: Claude Opus 5 (1M context) --- .../Harness/FileAccess/FileAccessProvider.cs | 2 +- .../Harness/FileStore/FileEditor.cs | 31 ++++++++++++++----- .../FileStore/FileSystemAgentFileStore.cs | 2 +- .../FileStore/InMemoryAgentFileStore.cs | 2 +- .../Harness/FileStore/FileEditorTests.cs | 16 ++++++++++ .../FileStore/InMemoryAgentFileStoreTests.cs | 19 ++++++++++++ 6 files changed, 61 insertions(+), 11 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI/Harness/FileAccess/FileAccessProvider.cs b/dotnet/src/Microsoft.Agents.AI/Harness/FileAccess/FileAccessProvider.cs index 72ad41156a..bd4c3c0621 100644 --- a/dotnet/src/Microsoft.Agents.AI/Harness/FileAccess/FileAccessProvider.cs +++ b/dotnet/src/Microsoft.Agents.AI/Harness/FileAccess/FileAccessProvider.cs @@ -322,7 +322,7 @@ private async Task ReadAsync(string fileName, CancellationToken cancella /// Thrown when either bound is not positive, when precedes /// , or when is past the last line. /// - [Description("Read part of a file by 1-based inclusive line number; omit end_line to read to the end of the file, and an end_line past the last line is clamped. Line numbers match file_access_grep and file_access_replace_lines. Each line is prefixed with its number and a tab; everything after that tab is verbatim, including the line's own terminator, so it can be reused as a file_access_replace_lines new_line.")] + [Description("Read part of a file by 1-based inclusive line number; omit endLine to read to the end of the file, and an endLine past the last line is clamped. Line numbers match file_access_grep and file_access_replace_lines. Each line is prefixed with its number and a tab; everything after that tab is verbatim, including the line's own terminator, so it can be reused as a file_access_replace_lines new_line.")] private async Task ReadLinesAsync(string fileName, int startLine, int? endLine = null, CancellationToken cancellationToken = default) { string path = StorePaths.NormalizeRelativePath(fileName); diff --git a/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileEditor.cs b/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileEditor.cs index a8fca086b9..294dce8d75 100644 --- a/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileEditor.cs +++ b/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileEditor.cs @@ -107,24 +107,26 @@ internal static List SliceLines(string content, int startLine, int? endL List lines = SplitLinesKeepEnds(content); int total = lines.Count; + // These messages reach the model as the tool's failure text, so they name the arguments as the + // generated schema exposes them (startLine/endLine), not in snake_case. if (startLine < 1) { - throw new ArgumentException($"start_line must be a positive integer, got {startLine}."); + throw new ArgumentException($"startLine must be a positive integer, got {startLine}."); } if (endLine is < 1) { - throw new ArgumentException($"end_line must be a positive integer, got {endLine}."); + throw new ArgumentException($"endLine must be a positive integer, got {endLine}."); } if (endLine < startLine) { - throw new ArgumentException($"end_line ({endLine}) must not be less than start_line ({startLine})."); + throw new ArgumentException($"endLine ({endLine}) must not be less than startLine ({startLine})."); } if (startLine > total) { - throw new ArgumentException($"start_line {startLine} is out of range (file has {total} lines)."); + throw new ArgumentException($"startLine {startLine} is out of range (file has {total} lines)."); } // Clamping end_line rather than failing keeps "read from here to the end" a single call. @@ -133,11 +135,24 @@ internal static List SliceLines(string content, int startLine, int? endL } /// - /// Returns without the trailing \n that terminates it, so search - /// patterns are matched against a line's text rather than its line break. + /// Returns without the \r\n, \n, or lone \r that + /// terminates it, so search patterns are matched against a line's text rather than its line break. /// - internal static string TrimTrailingNewline(string line) - => line.EndsWith("\n", StringComparison.Ordinal) ? line.Substring(0, line.Length - 1) : line; + /// + /// Leaving any part of the terminator in place would make an end-anchored pattern such as + /// match$ fail on a CRLF or lone-CR line whose text is exactly match. + /// + internal static string TrimLineTerminator(string line) + { + if (line.EndsWith("\r\n", StringComparison.Ordinal)) + { + return line.Substring(0, line.Length - 2); + } + + return line.EndsWith("\n", StringComparison.Ordinal) || line.EndsWith("\r", StringComparison.Ordinal) + ? line.Substring(0, line.Length - 1) + : line; + } private static int CountOccurrences(string content, string value) { diff --git a/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileSystemAgentFileStore.cs b/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileSystemAgentFileStore.cs index 12ba946c14..b52565f3ac 100644 --- a/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileSystemAgentFileStore.cs +++ b/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileSystemAgentFileStore.cs @@ -213,7 +213,7 @@ public override async Task> SearchAsync( for (int i = 0; i < lines.Count; i++) { - Match match = regex.Match(FileEditor.TrimTrailingNewline(lines[i])); + Match match = regex.Match(FileEditor.TrimLineTerminator(lines[i])); if (match.Success) { matchingLines.Add(new FileSearchMatch { LineNumber = i + 1, Line = lines[i] }); diff --git a/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/InMemoryAgentFileStore.cs b/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/InMemoryAgentFileStore.cs index bafd604837..10d1bc7b7b 100644 --- a/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/InMemoryAgentFileStore.cs +++ b/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/InMemoryAgentFileStore.cs @@ -152,7 +152,7 @@ public override Task> SearchAsync(string directo for (int i = 0; i < lines.Count; i++) { - Match match = regex.Match(FileEditor.TrimTrailingNewline(lines[i])); + Match match = regex.Match(FileEditor.TrimLineTerminator(lines[i])); if (match.Success) { matchingLines.Add(new FileSearchMatch { LineNumber = i + 1, Line = lines[i] }); diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileStore/FileEditorTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileStore/FileEditorTests.cs index 0ed63dfdec..adbc08bf8f 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileStore/FileEditorTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileStore/FileEditorTests.cs @@ -211,6 +211,22 @@ public void SplitLinesKeepEnds_ConcatenationRoundTripsTheContent() Assert.Equal(Content, string.Concat(lines)); } + [Theory] + [InlineData("match\r\n", "match")] + [InlineData("match\n", "match")] + [InlineData("match\r", "match")] + [InlineData("match", "match")] + [InlineData("", "")] + [InlineData("a\rb\n", "a\rb")] + public void TrimLineTerminator_RemovesOnlyTheTrailingTerminator(string line, string expected) + { + // Act + string trimmed = FileEditor.TrimLineTerminator(line); + + // Assert + Assert.Equal(expected, trimmed); + } + #endregion #region SliceLines diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileStore/InMemoryAgentFileStoreTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileStore/InMemoryAgentFileStoreTests.cs index 2d64c8401a..207e803150 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileStore/InMemoryAgentFileStoreTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileStore/InMemoryAgentFileStoreTests.cs @@ -265,6 +265,25 @@ public async Task SearchFiles_LoneCarriageReturn_SplitsLikeTheLineEditorAsync() Assert.Equal("beta match\r", results[0].MatchingLines[0].Line); } + [Theory] + [InlineData("alpha\r\nbeta match\r\ngamma\r\n")] + [InlineData("alpha\rbeta match\rgamma")] + [InlineData("alpha\nbeta match\ngamma\n")] + public async Task SearchFiles_EndAnchoredPatternMatchesRegardlessOfTerminatorAsync(string content) + { + // Arrange — the pattern anchors to the end of the line's text, which is "beta match". + var store = new InMemoryAgentFileStore(); + await store.WriteAsync("folder/notes.md", content); + + // Act + var results = await store.SearchAsync("folder", "match$"); + + // Assert — the terminator is not part of the text the pattern is matched against. + Assert.Single(results); + Assert.Single(results[0].MatchingLines); + Assert.Equal(2, results[0].MatchingLines[0].LineNumber); + } + [Fact] public async Task SearchFiles_SnippetIsAnchoredAtTheMatchAsync() {