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 9755e8ba6b7..892d346c4f4 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 02b15847e9b..2415efdb3be 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 7674688113c..df1ab52c4a9 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; @@ -136,6 +147,7 @@ These files persist beyond the current session and may be shared across sessions private readonly bool _disableWriteTools; private readonly bool _disableReadOnlyToolApproval; private readonly bool _disableWriteToolApproval; + private readonly bool _disableSearchAlignmentCheck; private readonly SemaphoreSlim _writeLock = new(1, 1); private AITool[]? _tools; @@ -157,11 +169,13 @@ public FileAccessProvider(AgentFileStore fileStore, FileAccessProviderOptions? o this._disableWriteTools = options?.DisableWriteTools ?? false; this._disableReadOnlyToolApproval = options?.DisableReadOnlyToolApproval ?? false; this._disableWriteToolApproval = options?.DisableWriteToolApproval ?? false; + this._disableSearchAlignmentCheck = options?.DisableSearchAlignmentCheck ?? false; } /// /// Gets an auto-approval rule that approves the read-only file access tools - /// (, , and ). + /// (, , , + /// and ). /// /// /// @@ -179,6 +193,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 +228,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) @@ -288,7 +304,7 @@ private async Task WriteAsync(string fileName, string content, bool over /// The name of the file to read. /// A token to cancel the operation. /// The file content or a not-found message. - [Description("Read the content of a file by name. Returns the file content or a message indicating the file was not found.")] + [Description("Read the content of a file by name. Returns the file content or a message indicating the file was not found. To edit by line number afterwards, count lines terminated by \\n, \\r\\n, or a lone \\r; each line keeps its own terminator, and content ending in a terminator has no extra empty line after it.")] private async Task ReadAsync(string fileName, CancellationToken cancellationToken = default) { string path = StorePaths.NormalizeRelativePath(fileName); @@ -296,6 +312,46 @@ 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. + /// + /// The line numbers agree with the ones file_access_grep reports, because + /// must number by — + /// the split this method and file_access_replace_lines use. A store overriding it owns that + /// numbering, so file_access_grep verifies it and refuses results that do not line up. + /// + /// + /// 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 endLine to read to the end of the file, and an endLine past the last line is clamped. 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. Line numbers count lines terminated by \\n, \\r\\n, or a lone \\r, and content ending in a terminator has no extra empty line after it.")] + 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. /// @@ -377,7 +433,7 @@ private async Task ReplaceAsync(string fileName, string oldString, strin /// The list of 1-based line numbers and their literal replacement text. /// A token to cancel the operation. /// A confirmation message including the number of lines replaced, or a failure message. - [Description("Replace lines in a file. Provide a list of edits, each with a 1-based line_number and a literal new_line (include your own trailing newline); an empty new_line deletes the line, including its line break. Fails on out-of-range or duplicate line numbers.")] + [Description("Replace lines in a file. Provide a list of edits, each with a 1-based line_number and a literal new_line (include your own trailing newline); an empty new_line deletes the line, including its line break. Fails on out-of-range or duplicate line numbers. Line numbers count lines terminated by \\n, \\r\\n, or a lone \\r; each line keeps its own terminator, and content ending in a terminator has no extra empty line after it.")] private async Task ReplaceLinesAsync(string fileName, List edits, CancellationToken cancellationToken = default) { await this._writeLock.WaitAsync(cancellationToken).ConfigureAwait(false); @@ -417,6 +473,7 @@ private async Task ReplaceLinesAsync(string fileName, List - '**' matches across subdirectories, so use \"**/*.md\" to match markdown files at any depth, or \"reports/**\" to restrict the search to the 'reports' subtree. Returns matching results whose file names are paths relative to the store root (usable with file_access_read), along with snippets and matching lines with line numbers. + Line numbers count lines terminated by \n, \r\n, or a lone \r, and content ending in a terminator has no extra empty line after it. """)] private async Task> GrepAsync(string regexPattern, string? globPattern = null, string? directory = null, CancellationToken cancellationToken = default) { @@ -424,6 +481,11 @@ private async Task> GrepAsync(string regexPattern, string string target = StorePaths.NormalizeRelativePath(directory ?? string.Empty, isDirectory: true); IReadOnlyList results = await this._fileStore.SearchAsync(target, regexPattern, pattern, recursive: true, cancellationToken).ConfigureAwait(false); + if (!this._disableSearchAlignmentCheck) + { + await SearchAlignment.ThrowIfMisalignedAsync(this._fileStore, target, results, regexPattern, cancellationToken).ConfigureAwait(false); + } + // store.SearchAsync returns FileName relative to the searched directory; re-root each result to the // store root so the names compose directly with file_access_read/replace/delete. string prefix = target; @@ -460,6 +522,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 8f8e406e486..876393f2aca 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. @@ -58,4 +59,18 @@ public sealed class FileAccessProviderOptions /// tools that modify the store are not exposed in that case. /// public bool DisableWriteToolApproval { get; set; } + + /// + /// Gets or sets a value indicating whether to skip the check that a store's reported + /// values address the same lines the line editor acts on. + /// + /// + /// The check only runs for a store that overrides without + /// declaring , and costs one extra read per + /// matched file. Turn it off only when the store's alignment is established some other + /// way: without it, a mis-numbered grep result reaches the model and an edit can land on the wrong + /// line silently. Prefer setting on the + /// store, which opts out one store rather than every store this provider is given. + /// + public bool DisableSearchAlignmentCheck { get; set; } } diff --git a/dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/FileMemoryProvider.cs b/dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/FileMemoryProvider.cs index 0b41e82fba8..a4d3d1fbb67 100644 --- a/dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/FileMemoryProvider.cs +++ b/dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/FileMemoryProvider.cs @@ -62,6 +62,12 @@ public sealed class FileMemoryProvider : AIContextProvider, IDisposable /// The name of the tool that replaces whole lines within a memory file. public const string ReplaceLinesToolName = "file_memory_replace_lines"; + // The file-access wording sends the model to read_lines, which this provider does not register. + private const string MisalignedMemoryMessage = + "This store's line numbers do not line up with the numbering used by file_memory_replace_lines, " + + "so editing by the reported numbers would change the wrong lines (or a file changed while the " + + "search ran). Use file_memory_read to locate the content before editing."; + private const string DescriptionSuffix = "_description.md"; private const string MemoryIndexFileName = "memories.md"; private const int MaxIndexEntries = 50; @@ -87,6 +93,7 @@ This ensures important data remains accessible across long-running sessions. private readonly ProviderSessionState _sessionState; private readonly SemaphoreSlim _writeLock = new(1, 1); private readonly string _instructions; + private readonly bool _disableSearchAlignmentCheck; private IReadOnlyList? _stateKeys; private AITool[]? _tools; @@ -107,6 +114,7 @@ public FileMemoryProvider(AgentFileStore fileStore, Func( stateInitializer ?? (_ => new FileMemoryState()), this.GetType().Name, @@ -215,7 +223,7 @@ private async Task WriteAsync(string fileName, string content, string? d /// The name of the file to read. /// A token to cancel the operation. /// The file content or a not-found message. - [Description("Read the content of a memory file by name. Returns the file content or a message indicating the file was not found.")] + [Description("Read the content of a memory file by name. Returns the file content or a message indicating the file was not found. To edit by line number afterwards, count lines terminated by \\n, \\r\\n, or a lone \\r; each line keeps its own terminator, and content ending in a terminator has no extra empty line after it.")] private async Task ReadAsync(string fileName, CancellationToken cancellationToken = default) { string normalized = StorePaths.NormalizeRelativePath(fileName); @@ -356,7 +364,7 @@ private async Task ReplaceAsync(string fileName, string oldString, strin /// The list of 1-based line numbers and their literal replacement text. /// A token to cancel the operation. /// A confirmation message including the number of lines replaced, or a failure message. - [Description("Replace lines in a memory file. Provide a list of edits, each with a 1-based line_number and a literal new_line (include your own trailing newline); an empty new_line deletes the line, including its line break. Fails on out-of-range or duplicate line numbers.")] + [Description("Replace lines in a memory file. Provide a list of edits, each with a 1-based line_number and a literal new_line (include your own trailing newline); an empty new_line deletes the line, including its line break. Fails on out-of-range or duplicate line numbers. Line numbers count lines terminated by \\n, \\r\\n, or a lone \\r; each line keeps its own terminator, and content ending in a terminator has no extra empty line after it.")] private async Task ReplaceLinesAsync(string fileName, List edits, CancellationToken cancellationToken = default) { string normalized = StorePaths.NormalizeRelativePath(fileName); @@ -401,6 +409,12 @@ private async Task> GrepAsync(string regexPattern, string string? pattern = string.IsNullOrWhiteSpace(globPattern) ? null : globPattern; IReadOnlyList results = await this._fileStore.SearchAsync(state.WorkingFolder, regexPattern, pattern, recursive: false, cancellationToken).ConfigureAwait(false); + if (!this._disableSearchAlignmentCheck) + { + await SearchAlignment.ThrowIfMisalignedAsync( + this._fileStore, state.WorkingFolder, results, regexPattern, cancellationToken, MisalignedMemoryMessage).ConfigureAwait(false); + } + // Filter out internal files (description sidecars and memory index) so they stay hidden. var filtered = new List(results.Count); foreach (var result in results) diff --git a/dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/FileMemoryProviderOptions.cs b/dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/FileMemoryProviderOptions.cs index be28af694db..83b7bea8d13 100644 --- a/dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/FileMemoryProviderOptions.cs +++ b/dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/FileMemoryProviderOptions.cs @@ -15,4 +15,18 @@ public sealed class FileMemoryProviderOptions /// that guide the agent on how to use file-based memory effectively. /// public string? Instructions { get; set; } + + /// + /// Gets or sets a value indicating whether to skip the check that a store's reported + /// values address the same lines the line editor acts on. + /// + /// + /// The check only runs for a store that overrides without + /// declaring , and costs one extra read per + /// matched file. Turn it off only when the store's alignment is established some other + /// way: without it, a mis-numbered grep result reaches the model and an edit can land on the wrong + /// line silently. Prefer setting on the + /// store, which opts out one store rather than every store this provider is given. + /// + public bool DisableSearchAlignmentCheck { get; set; } } diff --git a/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/AgentFileStore.cs b/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/AgentFileStore.cs index 4a7feee0517..f8ca614fa91 100644 --- a/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/AgentFileStore.cs +++ b/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/AgentFileStore.cs @@ -1,11 +1,14 @@ // Copyright (c) Microsoft. All rights reserved. +using System; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; +using System.Text.RegularExpressions; using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.FileSystemGlobbing; using Microsoft.Shared.DiagnosticIds; +using Microsoft.Shared.Diagnostics; namespace Microsoft.Agents.AI; @@ -92,7 +95,198 @@ public abstract class AgentFileStore /// A list of search results. Each result's is the matching file's /// path relative to . /// - public abstract Task> SearchAsync(string directory, string regexPattern, string? globPattern = null, bool recursive = false, CancellationToken cancellationToken = default); + public virtual async Task> SearchAsync(string directory, string regexPattern, string? globPattern = null, bool recursive = false, CancellationToken cancellationToken = default) + { + // Compile with a match timeout to guard against catastrophic backtracking (ReDoS). + var regex = new Regex(regexPattern, RegexOptions.IgnoreCase, TimeSpan.FromSeconds(5)); + IReadOnlyList names = await this.FindMatchingFilesAsync(directory, regexPattern, globPattern, recursive, cancellationToken).ConfigureAwait(false); + Matcher? matcher = globPattern is not null ? StorePaths.CreateGlobMatcher(globPattern) : null; + var results = new List(); + + foreach (string name in names) + { + cancellationToken.ThrowIfCancellationRequested(); + + // Re-apply the caller's scope: FindMatchingFilesAsync is explicitly allowed to + // over-return, and must not be able to widen what the caller asked for. + if (!StorePaths.MatchesGlob(name, matcher) || + (!recursive && name.IndexOf("/", StringComparison.Ordinal) >= 0)) + { + continue; + } + + string path = string.IsNullOrEmpty(directory) ? name : $"{directory.TrimEnd('/')}/{name}"; + string? content = await this.ReadAsync(path, cancellationToken).ConfigureAwait(false); + if (content is null) + { + continue; // Deleted between enumeration and read. + } + + FileSearchResult? result = ScanContent(name, content, regex); + if (result is not null) + { + results.Add(result); + } + } + + // Tagged so the file-access tools can tell the base implementation numbered these results, + // without reflecting over the store's type (not trim-safe). Per call rather than per + // instance: a store that defers to base.SearchAsync only sometimes must not buy permanent + // trust for the results it numbers itself. + return new BaseSearchResults(results); + } + + /// + /// Gets the names of the files that may contain a match for . + /// + /// + /// + /// This is the hook a store uses to narrow the search to the files worth reading. Semantics are + /// deliberately a superset: returning a file that turns out not to match is harmless, + /// because re-scans every candidate, while omitting one loses the match. + /// A backend with a native search index should override this and push + /// down to it, widening rather than guessing where the dialect + /// cannot express the pattern. + /// + /// + /// The default implementation has no index to narrow with, so it walks + /// and returns every file in scope. Overriding + /// instead is also supported, but then line numbering is the store's + /// responsibility (see ) and the file-access tools verify it. + /// + /// + /// The relative directory being searched. Use an empty string for the root. + /// The pattern was called with, as a hint. + /// The optional glob, matched against each file's path relative to . + /// When only direct children are in scope. + /// A token to cancel the operation. + /// File paths relative to , using forward slashes. + protected virtual async Task> FindMatchingFilesAsync(string directory, string regexPattern, string? globPattern = null, bool recursive = false, CancellationToken cancellationToken = default) + { + _ = regexPattern; // No index to narrow with here; a backend with one overrides this. + var names = new List(); + var pending = new Stack(); + pending.Push(string.Empty); + + while (pending.Count > 0) + { + string relativeDir = pending.Pop(); + string target = string.IsNullOrEmpty(relativeDir) + ? directory + : (string.IsNullOrEmpty(directory) ? relativeDir : $"{directory.TrimEnd('/')}/{relativeDir}"); + + foreach (FileStoreEntry entry in await this.ListChildrenAsync(target, cancellationToken).ConfigureAwait(false)) + { + string child = string.IsNullOrEmpty(relativeDir) ? entry.Name : $"{relativeDir}/{entry.Name}"; + if (entry.Type == FileStoreEntry.Directory) + { + if (recursive) + { + pending.Push(child); + } + } + else + { + names.Add(child); + } + } + } + + return names; + } + + /// + /// Splits into the lines this SDK's line numbers address. + /// + /// + /// + /// This is the published definition of a line for the whole file-access surface: the + /// read_lines and replace_lines tools, and every + /// reported by , are coordinates in this list. Each line keeps its + /// terminator (\r\n, \n, or a lone \r), and the final line has none when the + /// content does not end with a newline. + /// + /// + /// A store that overrides must number its matches by this split, + /// otherwise grep and the line editor disagree and an edit lands on the wrong line. The rule is + /// per-SDK: it is not required to match the Python implementation, only to be consistent within + /// this one, because a line number never crosses runtimes. + /// + /// + /// The full text to split. + /// The lines, each with its terminator attached. + public static IReadOnlyList SplitLines(string content) => FileEditor.SplitLinesKeepEnds(Throw.IfNull(content)); + + /// + /// Finds every line of matching , numbered by + /// . + /// + /// + /// This is the numbering primitive uses, published so a store that + /// supplies its own can produce aligned results rather than re-deriving + /// them. Lines are reported verbatim, terminator included; the pattern is matched against the + /// line without its terminator, so an end-anchored pattern behaves the same on CRLF content. + /// + /// The name recorded on the result, relative to the searched directory. + /// The file's full text. + /// A compiled pattern, normally from the same source string passed to . + /// The match metadata, or when no line matches. + public static FileSearchResult? ScanContent(string fileName, string content, Regex regex) + { + _ = Throw.IfNull(content); + _ = Throw.IfNull(regex); + + IReadOnlyList lines = SplitLines(content); + var matchingLines = new List(); + string? firstSnippet = null; + int lineStartOffset = 0; + + for (int i = 0; i < lines.Count; i++) + { + // Match over the line's text only, without copying it out of the line. + Match match = regex.Match(lines[i], 0, FileEditor.LineContentLength(lines[i])); + if (match.Success) + { + matchingLines.Add(new FileSearchMatch { LineNumber = i + 1, Line = lines[i] }); + + // Build a context snippet around the first match (+/-50 chars). + if (firstSnippet is null) + { + int charIndex = lineStartOffset + match.Index; + int snippetStart = Math.Max(0, charIndex - 50); + int snippetEnd = Math.Min(content.Length, charIndex + match.Value.Length + 50); + firstSnippet = content.Substring(snippetStart, snippetEnd - snippetStart); + } + } + + // Advance past this line; its terminator is already part of its length. + lineStartOffset += lines[i].Length; + } + + return matchingLines.Count == 0 + ? null + : new FileSearchResult { FileName = fileName, Snippet = firstSnippet!, MatchingLines = matchingLines }; + } + + /// + /// Gets a value indicating whether this store guarantees its + /// values are coordinates in . + /// + /// + /// + /// Set by a store that overrides and numbers lines correctly — normally + /// because it reports through . Declaring it opts the store out of the + /// alignment check the file-access tools otherwise run on every grep, which costs one extra read + /// per matched file. A store that does not override need not + /// set it: the base implementation is aligned by construction and is never checked. + /// + /// + /// This is a promise, not a hint. Declaring it while numbering lines differently reinstates + /// exactly the failure the check exists to catch — replace_lines silently editing the + /// wrong line — so only set it if a test pins the alignment. + /// + /// + public virtual bool ReportsAlignedLineNumbers => false; /// /// Ensures a directory exists, creating it if necessary. diff --git a/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/BaseSearchResults.cs b/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/BaseSearchResults.cs new file mode 100644 index 00000000000..6817b3390fd --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/BaseSearchResults.cs @@ -0,0 +1,86 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Collections.Generic; + +namespace Microsoft.Agents.AI; + +/// +/// Marks a result set as having been numbered by the base . +/// +/// +/// +/// The alignment check needs to know whether the line numbers it is about to hand to the model came +/// from the base implementation (aligned by construction) or from a store's own +/// . Reflecting over the store's type is not trim-safe, and a +/// flag on the store would be per instance rather than per call — a store that defers to +/// base.SearchAsync only sometimes would buy permanent trust for the results it numbers itself, +/// which is exactly the failure the check exists to catch. +/// +/// +/// Tagging the returned list keeps the signal with the data, but the tag alone only says who built +/// the list, not that its contents are still theirs: the list and every +/// in it are mutable, so an override could renumber them in place and keep the tag. The numbers are +/// therefore snapshotted at construction and re-checked by , so any edit -- +/// or a rebuilt list, which loses the tag outright -- falls back to verification rather than trust. +/// +/// +internal sealed class BaseSearchResults : List +{ + private readonly (string FileName, int[] LineNumbers)[] _numbered; + + internal BaseSearchResults(IReadOnlyList results) + : base(results) + { + this._numbered = new (string FileName, int[] LineNumbers)[results.Count]; + for (int i = 0; i < results.Count; i++) + { + FileSearchResult result = results[i]; + int[] lineNumbers = new int[result.MatchingLines.Count]; + for (int j = 0; j < lineNumbers.Length; j++) + { + lineNumbers[j] = result.MatchingLines[j].LineNumber; + } + + this._numbered[i] = (result.FileName, lineNumbers); + } + } + + /// + /// Returns whether the file names and line numbers still match what the base implementation produced. + /// + /// + /// is deliberately not covered: a store may report the text + /// differently, which is why the alignment check matches by pattern rather than by string. + /// + internal bool IsUnmodified() + { + if (this.Count != this._numbered.Length) + { + return false; + } + + for (int i = 0; i < this.Count; i++) + { + FileSearchResult result = this[i]; + (string fileName, int[] lineNumbers) = this._numbered[i]; + if (result is null || + !string.Equals(result.FileName, fileName, StringComparison.Ordinal) || + result.MatchingLines is null || + result.MatchingLines.Count != lineNumbers.Length) + { + return false; + } + + for (int j = 0; j < lineNumbers.Length; j++) + { + if (result.MatchingLines[j].LineNumber != lineNumbers[j]) + { + return false; + } + } + } + + return true; + } +} diff --git a/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileEditor.cs b/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileEditor.cs index 32c3ff84783..0d267c85d4f 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 { @@ -80,6 +81,20 @@ 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; + + // 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($"startLine must be a positive integer, got {startLine}."); + } + + if (endLine is < 1) + { + throw new ArgumentException($"endLine must be a positive integer, got {endLine}."); + } + + if (endLine < startLine) + { + throw new ArgumentException($"endLine ({endLine}) must not be less than startLine ({startLine})."); + } + + if (startLine > total) + { + 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. + int lastLine = endLine is null ? total : Math.Min(endLine.Value, total); + return lines.GetRange(startLine - 1, lastLine - startLine + 1); + } + + /// + /// Returns without its trailing \r\n, \n or lone \r. + /// + internal static string TrimLineTerminator(string line) => line.Substring(0, LineContentLength(line)); + + /// + /// Returns the length of up to but excluding 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. + /// + /// + /// Leaving any part of the terminator in range would make an end-anchored pattern such as + /// match$ fail on a CRLF or lone-CR line whose text is exactly match. This returns a + /// length rather than a trimmed string because the callers scan every line before knowing which ones + /// match, and copying each one would duplicate nearly the whole file on every search. + /// + internal static int LineContentLength(string line) + { + if (line.EndsWith("\r\n", StringComparison.Ordinal)) + { + return line.Length - 2; + } + + return line.EndsWith("\n", StringComparison.Ordinal) || line.EndsWith("\r", StringComparison.Ordinal) + ? line.Length - 1 + : line.Length; + } + private static int CountOccurrences(string content, string value) { int count = 0; @@ -109,7 +193,14 @@ 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 line-edit tools and by the + /// implementations in this package, so for those stores the line numbers + /// reported by grep address the same lines that replace_lines edits. A custom store + /// supplies its own , which must number by this split but + /// does not inherit it, so the file-access tools verify it. + /// + internal static List SplitLinesKeepEnds(string content) { var lines = new List(); int start = 0; diff --git a/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileLineEdit.cs b/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileLineEdit.cs index 5100715bdf3..5893e5af9e3 100644 --- a/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileLineEdit.cs +++ b/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileLineEdit.cs @@ -28,4 +28,13 @@ public sealed class FileLineEdit [JsonPropertyName("new_line")] [Description("Literal replacement text for the line, including any trailing newline you want to keep (the editor does not add one). Set to an empty string to delete the line entirely, including its line break.")] public string NewLine { get; set; } = string.Empty; + + /// + /// Gets or sets the text the caller believes is currently on that line. When set, the edit is + /// rejected unless it matches, which catches an out-of-date line number or a file that changed + /// since it was read. The trailing line terminator is ignored in the comparison. + /// + [JsonPropertyName("expected_line")] + [Description("Optional: the text you believe is currently on that line, as reported by grep or read_lines. When supplied, the edit is rejected unless it matches, which catches an out-of-date line number or a file that changed since you looked. The trailing newline is ignored in the comparison.")] + public string? ExpectedLine { get; set; } } diff --git a/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileSearchMatch.cs b/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileSearchMatch.cs index 0bf2d102d3a..af40a2c4818 100644 --- a/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileSearchMatch.cs +++ b/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileSearchMatch.cs @@ -19,8 +19,16 @@ 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. /// + /// + /// For the implementations in this package, 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. A custom store filling this + /// type from its own may report the text differently, but not + /// the number: that must address . + /// [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 8f3d171c94d..2e4219c4169 100644 --- a/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileSystemAgentFileStore.cs +++ b/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/FileSystemAgentFileStore.cs @@ -161,6 +161,13 @@ public override Task FileExistsAsync(string path, CancellationToken cancel return Task.FromResult(File.Exists(fullPath)); } + /// + /// + /// This store reports through , so its line numbers are + /// coordinates in by construction. + /// + public override bool ReportsAlignedLineNumbers => true; + /// public override async Task> SearchAsync( string directory, @@ -203,41 +210,12 @@ public override async Task> SearchAsync( } #endif - // Search each line for regex matches, tracking line numbers and building a snippet. - string[] lines = fileContent.Split('\n'); - var matchingLines = new List(); - string? firstSnippet = null; - int lineStartOffset = 0; - - for (int i = 0; i < lines.Length; i++) - { - Match match = regex.Match(lines[i]); - if (match.Success) - { - matchingLines.Add(new FileSearchMatch { LineNumber = i + 1, Line = lines[i].TrimEnd('\r') }); - - // Build a context snippet around the first match (±50 chars). - if (firstSnippet is null) - { - int charIndex = lineStartOffset + match.Index; - int snippetStart = Math.Max(0, charIndex - 50); - int snippetEnd = Math.Min(fileContent.Length, charIndex + match.Value.Length + 50); - firstSnippet = fileContent.Substring(snippetStart, snippetEnd - snippetStart); - } - } - - // Advance the offset past this line (including the '\n' separator). - lineStartOffset += lines[i].Length + 1; - } - - if (matchingLines.Count > 0) + // Number the lines through the base class's published primitive, so this store + // and the line editor cannot drift apart. + FileSearchResult? result = ScanContent(relativeName, fileContent, regex); + if (result is not null) { - results.Add(new FileSearchResult - { - FileName = relativeName, - Snippet = firstSnippet!, - MatchingLines = matchingLines, - }); + results.Add(result); } } diff --git a/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/InMemoryAgentFileStore.cs b/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/InMemoryAgentFileStore.cs index 62dfc020cb4..680a4848e90 100644 --- a/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/InMemoryAgentFileStore.cs +++ b/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/InMemoryAgentFileStore.cs @@ -103,6 +103,13 @@ public override Task FileExistsAsync(string path, CancellationToken cancel return Task.FromResult(this._files.ContainsKey(path)); } + /// + /// + /// This store reports through , so its line numbers are + /// coordinates in by construction. + /// + public override bool ReportsAlignedLineNumbers => true; + /// public override Task> SearchAsync(string directory, string regexPattern, string? globPattern = null, bool recursive = false, CancellationToken cancellationToken = default) { @@ -141,42 +148,12 @@ public override Task> SearchAsync(string directo continue; } - // Search each line for regex matches, tracking line numbers and building a snippet. - string fileContent = kvp.Value; - string[] lines = fileContent.Split('\n'); - var matchingLines = new List(); - string? firstSnippet = null; - int lineStartOffset = 0; - - for (int i = 0; i < lines.Length; i++) - { - Match match = regex.Match(lines[i]); - if (match.Success) - { - matchingLines.Add(new FileSearchMatch { LineNumber = i + 1, Line = lines[i].TrimEnd('\r') }); - - // Build a context snippet around the first match (±50 chars). - if (firstSnippet is null) - { - int charIndex = lineStartOffset + match.Index; - int snippetStart = Math.Max(0, charIndex - 50); - int snippetEnd = Math.Min(fileContent.Length, charIndex + match.Value.Length + 50); - firstSnippet = fileContent.Substring(snippetStart, snippetEnd - snippetStart); - } - } - - // Advance the offset past this line (including the '\n' separator). - lineStartOffset += lines[i].Length + 1; - } - - if (matchingLines.Count > 0) + // Number the lines through the base class's published primitive, so this store + // and the line editor cannot drift apart. + FileSearchResult? result = ScanContent(relativeName, kvp.Value, regex); + if (result is not null) { - results.Add(new FileSearchResult - { - FileName = relativeName, - Snippet = firstSnippet!, - MatchingLines = matchingLines, - }); + results.Add(result); } } diff --git a/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/SearchAlignment.cs b/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/SearchAlignment.cs new file mode 100644 index 00000000000..729bca666c3 --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI/Harness/FileStore/SearchAlignment.cs @@ -0,0 +1,129 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Collections.Generic; +using System.Text.RegularExpressions; +using System.Threading; +using System.Threading.Tasks; + +namespace Microsoft.Agents.AI; + +/// +/// Checks that a store's reported line numbers address the same lines the line editor will edit. +/// +/// +/// +/// grep takes its values from the store, while +/// read_lines and replace_lines re-count them from . +/// A store that overrides and numbers lines differently makes +/// those two disagree, and an edit then lands on the wrong line — in range, reporting success. +/// +/// +/// The check is skipped for a store that uses the base +/// (aligned by construction) or declares . +/// Otherwise every reported number is re-checked by running the pattern against that line of +/// , which is what replace_lines indexes into. Matching +/// by pattern rather than by string equality is deliberate: a custom store is not required to report +/// the line verbatim with its terminator, so comparing text would reject correct stores. +/// +/// +/// This is detection, not proof — a pattern matching every line (.) passes even if the +/// numbering is skewed — and it is deliberately whole-call: one skewed file means the store's +/// coordinates cannot be trusted anywhere. +/// +/// +internal static class SearchAlignment +{ + internal const string MisalignedMessage = + "This store's line numbers do not line up with the numbering used by read_lines and " + + "replace_lines, so editing by the reported numbers would change the wrong lines (or a file " + + "changed while the search ran). Use read or read_lines to locate the content before editing."; + + /// + /// Throws when cannot be trusted to address the editor's lines. + /// + internal static async Task ThrowIfMisalignedAsync( + AgentFileStore store, + string directory, + IReadOnlyList results, + string regexPattern, + CancellationToken cancellationToken, + string? misalignedMessage = null, // The memory provider passes its own; it registers no read_lines. + TimeSpan? matchTimeout = null) // Overridable so a test can reach the timeout path. + { + if (results.Count == 0 || IsTrusted(store, results)) + { + return; + } + + string failure = misalignedMessage ?? MisalignedMessage; + + // A store that supplies its own SearchAsync may have matched with a different engine or + // dialect. If the pattern will not compile here there is nothing to check against, and + // failing to verify is not the same as finding a mismatch -- the store already accepted it. + Regex regex; + try + { + regex = new Regex(regexPattern, RegexOptions.IgnoreCase, matchTimeout ?? TimeSpan.FromSeconds(5)); + } + catch (ArgumentException) + { + return; + } + + foreach (FileSearchResult result in results) + { + string path = string.IsNullOrEmpty(directory) + ? result.FileName + : $"{directory.TrimEnd('/')}/{result.FileName}"; + + string? content = await store.ReadAsync(path, cancellationToken).ConfigureAwait(false); + if (content is null) + { + // Deleted between the search and this check. The base SearchAsync treats the same + // race as benign, so it must not be reported here as a store-correctness fault. + continue; + } + + IReadOnlyList lines = AgentFileStore.SplitLines(content); + foreach (FileSearchMatch match in result.MatchingLines) + { + // Both bounds: 0 or negative would index out of range below instead of reporting misalignment. + if (match.LineNumber < 1 || match.LineNumber > lines.Count) + { + throw new InvalidOperationException(failure); + } + + string line = lines[match.LineNumber - 1]; + bool matched; + try + { + matched = regex.Match(line, 0, FileEditor.LineContentLength(line)).Success; + } + catch (RegexMatchTimeoutException) + { + // Skip this match only; returning would leave every later result unchecked but reported. + continue; + } + + if (!matched) + { + throw new InvalidOperationException(failure); + } + } + } + } + + /// + /// Returns whether the store's line numbers are known to be aligned without checking. + /// + private static bool IsTrusted(AgentFileStore store, IReadOnlyList results) + { + // Trusted when the store declares alignment, or when these particular results carry the + // base implementation's tag -- which is per call, so an earlier delegation to + // base.SearchAsync cannot vouch for results the store numbered itself. The tag says who + // built the list; IsUnmodified says the numbers in it are still theirs. + return store.ReportsAlignedLineNumbers || + (results is BaseSearchResults tagged && tagged.IsUnmodified()); + } +} diff --git a/dotnet/tests/Microsoft.Agents.AI.Harness.UnitTests/HarnessAgentTests.cs b/dotnet/tests/Microsoft.Agents.AI.Harness.UnitTests/HarnessAgentTests.cs index b01af37e0fe..feebac05f85 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 e8797a4bc9b..df66a11852c 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/FileMemory/FileMemoryProviderTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileMemory/FileMemoryProviderTests.cs index 77dc5c7c4b7..7927161346f 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileMemory/FileMemoryProviderTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileMemory/FileMemoryProviderTests.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.Linq; using System.Text.Json; +using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.AI; using Moq; @@ -973,14 +974,74 @@ public async Task Write_WithDescription_ReturnsWrittenWithDescriptionAsync() #endregion + [Fact] + public async Task GrepRefusal_NamesMemoryToolsNotFileAccessToolsAsync() + { + // Arrange: the refusal reaches the model, and this provider registers no read_lines tool. + var store = new SkewedMemoryStore(); + await store.WriteAsync("notes.md", "alpha\nkeep me\n"); + var (tools, _, session) = await CreateToolsAsync(store); + var grep = GetTool(tools, "file_memory_grep"); + + // Act + Exception error = await Assert.ThrowsAnyAsync( + () => InvokeWithRunContextAsync(grep, new AIFunctionArguments { ["regexPattern"] = "keep me" }, session)); + + // Assert + string message = (error.InnerException ?? error).Message; + Assert.Contains("file_memory_read", message, StringComparison.Ordinal); + Assert.DoesNotContain("file_access", message, StringComparison.Ordinal); + } + #region Helper Methods - private static FileMemoryProvider CreateProvider(InMemoryAgentFileStore? store = null, Func? stateInitializer = null) + /// Numbers lines its own way, so the alignment check has something to catch. + private sealed class SkewedMemoryStore : AgentFileStore + { + private readonly Dictionary _files = []; + + public override Task WriteAsync(string path, string content, CancellationToken cancellationToken = default) + { + this._files[path] = content; + return Task.CompletedTask; + } + + public override Task ReadAsync(string path, CancellationToken cancellationToken = default) + => Task.FromResult(this._files.TryGetValue(path, out string? value) ? value : null); + + public override Task DeleteAsync(string path, CancellationToken cancellationToken = default) + => Task.FromResult(this._files.Remove(path)); + + public override Task> ListChildrenAsync(string directory, CancellationToken cancellationToken = default) + => Task.FromResult>( + this._files.Keys.Select(k => new FileStoreEntry(k, FileStoreEntry.File)).ToList()); + + public override Task FileExistsAsync(string path, CancellationToken cancellationToken = default) + => Task.FromResult(this._files.ContainsKey(path)); + + public override Task CreateDirectoryAsync(string path, CancellationToken cancellationToken = default) + => Task.CompletedTask; + + public override Task> SearchAsync(string directory, string regexPattern, string? globPattern = null, bool recursive = false, CancellationToken cancellationToken = default) + => Task.FromResult>( + [ + new FileSearchResult + { + FileName = "notes.md", + Snippet = string.Empty, + + // Deliberately wrong: the match is on line 2. + MatchingLines = [new FileSearchMatch { LineNumber = 1, Line = regexPattern }], + } + ]); + } + + private static FileMemoryProvider CreateProvider(AgentFileStore? store = null, Func? stateInitializer = null) { return new FileMemoryProvider(store ?? new InMemoryAgentFileStore(), stateInitializer); } - private static async Task<(IEnumerable Tools, FileMemoryState State, AgentSession Session)> CreateToolsAsync(InMemoryAgentFileStore? store = null, Func? stateInitializer = null) + private static async Task<(IEnumerable Tools, FileMemoryState State, AgentSession Session)> CreateToolsAsync(AgentFileStore? store = null, Func? stateInitializer = null) { var provider = CreateProvider(store, stateInitializer); var agent = new Mock().Object; diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileStore/AgentFileStoreContractTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileStore/AgentFileStoreContractTests.cs new file mode 100644 index 00000000000..6c4fe1a89b9 --- /dev/null +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileStore/AgentFileStoreContractTests.cs @@ -0,0 +1,416 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Text.RegularExpressions; +using System.Threading; +using System.Threading.Tasks; + +namespace Microsoft.Agents.AI.UnitTests.Harness.FileMemory; + +/// +/// Unit tests for the line-numbering contract on : the published split, +/// the numbering primitive, the narrowing hook, the base +/// built on top of them, and the guards that keep a store's line numbers honest. +/// +public class AgentFileStoreContractTests +{ + private const string Needle = "keep me"; + private const string Pathological = "(a+)+b"; + + /// + /// A store implementing only the mandatory members. Before the contract this could not exist: + /// SearchAsync was abstract. It now inherits the base implementation and must produce line + /// numbers that address the same lines the editor edits. + /// + private class ContentOnlyStore : AgentFileStore + { + public Dictionary Files { get; } = []; + + public override Task WriteAsync(string path, string content, CancellationToken cancellationToken = default) + { + this.Files[path] = content; + return Task.CompletedTask; + } + + public override Task ReadAsync(string path, CancellationToken cancellationToken = default) + => Task.FromResult(this.Files.TryGetValue(path, out string? value) ? value : null); + + public override Task DeleteAsync(string path, CancellationToken cancellationToken = default) + => Task.FromResult(this.Files.Remove(path)); + + public override Task> ListChildrenAsync(string directory, CancellationToken cancellationToken = default) + { + string prefix = string.IsNullOrEmpty(directory) ? string.Empty : directory + "/"; + var seen = new Dictionary(); + foreach (string path in this.Files.Keys) + { + if (!path.StartsWith(prefix, StringComparison.Ordinal)) + { + continue; + } + + string tail = path.Substring(prefix.Length); + int slash = tail.IndexOf('/'); + seen[slash < 0 ? tail : tail.Substring(0, slash)] = slash < 0 ? FileStoreEntry.File : FileStoreEntry.Directory; + } + + return Task.FromResult>( + seen.Select(kvp => new FileStoreEntry(kvp.Key, kvp.Value)).ToList()); + } + + public override Task FileExistsAsync(string path, CancellationToken cancellationToken = default) + => Task.FromResult(this.Files.ContainsKey(path)); + + public override Task CreateDirectoryAsync(string path, CancellationToken cancellationToken = default) + => Task.CompletedTask; + } + + /// A store whose narrowing hook consults an index instead of listing everything. + private sealed class NarrowingStore : ContentOnlyStore + { + public HashSet Indexed { get; } = []; + + protected override Task> FindMatchingFilesAsync(string directory, string regexPattern, string? globPattern = null, bool recursive = false, CancellationToken cancellationToken = default) + => Task.FromResult>(this.Indexed.OrderBy(x => x, StringComparer.Ordinal).ToList()); + } + + /// Numbers lines with its own rule, disagreeing with the editor. + private class SkewedStore : ContentOnlyStore + { + public override Task> SearchAsync(string directory, string regexPattern, string? globPattern = null, bool recursive = false, CancellationToken cancellationToken = default) + => Task.FromResult>( + [ + new FileSearchResult + { + FileName = "cfg.txt", + Snippet = string.Empty, + + // Deliberately wrong: the match is on line 2. + MatchingLines = [new FileSearchMatch { LineNumber = 1, Line = Needle }], + } + ]); + } + + /// The skewed store, declaring alignment it does not have. Pins the documented hazard. + private sealed class LyingStore : SkewedStore + { + public override bool ReportsAlignedLineNumbers => true; + } + + /// Reports a line number below the first line. + private sealed class ZeroLineStore : ContentOnlyStore + { + public override Task> SearchAsync(string directory, string regexPattern, string? globPattern = null, bool recursive = false, CancellationToken cancellationToken = default) + => Task.FromResult>( + [ + new FileSearchResult + { + FileName = "cfg.txt", + Snippet = string.Empty, + MatchingLines = [new FileSearchMatch { LineNumber = 0, Line = Needle }], + } + ]); + } + + /// Two results: the first line backtracks past the budget, the second is misnumbered. + private sealed class TrapThenSkewedStore : ContentOnlyStore + { + public override Task> SearchAsync(string directory, string regexPattern, string? globPattern = null, bool recursive = false, CancellationToken cancellationToken = default) + => Task.FromResult>( + [ + new FileSearchResult { FileName = "trap.txt", Snippet = string.Empty, MatchingLines = [new FileSearchMatch { LineNumber = 1, Line = string.Empty }] }, + new FileSearchResult { FileName = "cfg.txt", Snippet = string.Empty, MatchingLines = [new FileSearchMatch { LineNumber = 1, Line = string.Empty }] }, + ]); + } + + /// Delegates to the base, then renumbers the results it was handed, in place. + private sealed class RenumberingStore : ContentOnlyStore + { + public override async Task> SearchAsync(string directory, string regexPattern, string? globPattern = null, bool recursive = false, CancellationToken cancellationToken = default) + { + IReadOnlyList results = await base.SearchAsync(directory, regexPattern, globPattern, recursive, cancellationToken); + foreach (FileSearchResult result in results) + { + foreach (FileSearchMatch match in result.MatchingLines) + { + // Deliberately wrong: the match is on line 2. The list itself is not rebuilt. + match.LineNumber = 1; + } + } + + return results; + } + } + + [Fact] + public void SplitLines_PublishesTheEditorsRule() + { + foreach (string content in new[] { "a\nb\n", "a\rb", "", "x", "a\r\nb\r\n" }) + { + // Assert + Assert.Equal(FileEditor.SplitLinesKeepEnds(content), AgentFileStore.SplitLines(content)); + } + } + + [Fact] + public void ScanContent_NumbersBySplitLines() + { + // Act + FileSearchResult? result = AgentFileStore.ScanContent("f.txt", "alpha\r\nbeta match\r\ngamma\r\n", new Regex("match", RegexOptions.IgnoreCase)); + + // Assert + Assert.NotNull(result); + FileSearchMatch match = result!.MatchingLines[0]; + Assert.Equal(AgentFileStore.SplitLines("alpha\r\nbeta match\r\ngamma\r\n")[match.LineNumber - 1], match.Line); + Assert.Equal("beta match\r\n", match.Line); + } + + [Fact] + public async Task StoreWithoutSearch_UsesBasePathAndStaysAlignedAsync() + { + // Arrange + var store = new ContentOnlyStore(); + const string Raw = "alpha\r\nDEBUG = 1\r\nkeep me\r\nDEBUG = 2\r\n"; + await store.WriteAsync("cfg.txt", Raw); + + // Act + IReadOnlyList results = await store.SearchAsync(string.Empty, Needle, recursive: true); + + // Assert: the number grep reports addresses the line the editor will touch. + FileSearchMatch match = Assert.Single(Assert.Single(results).MatchingLines); + Assert.Equal(3, match.LineNumber); + Assert.Equal("keep me\r\n", match.Line); + Assert.Equal(match.Line, FileEditor.SliceLines(Raw, match.LineNumber, match.LineNumber)[0]); + } + + [Fact] + public async Task BaseSearch_ReappliesGlobAndRecursionWhenAStoreOverReturnsAsync() + { + // Arrange: the hook returns everything, ignoring both the glob and the recursion flag. + var store = new NarrowingStore(); + await store.WriteAsync("top.md", Needle); + await store.WriteAsync("notes.txt", Needle); + await store.WriteAsync("nested/deep.md", Needle); + foreach (string name in store.Files.Keys) + { + store.Indexed.Add(name); + } + + // Act + IReadOnlyList topLevelMarkdown = await store.SearchAsync(string.Empty, Needle, "*.md", recursive: true); + IReadOnlyList allMarkdown = await store.SearchAsync(string.Empty, Needle, "**/*.md", recursive: true); + IReadOnlyList shallow = await store.SearchAsync(string.Empty, Needle, recursive: false); + + // Assert: the glob is re-applied, using this SDK's Matcher semantics where "*" does not + // cross "/" (unlike the Python side's fnmatch, where it does). + Assert.Equal(["top.md"], topLevelMarkdown.Select(r => r.FileName)); + Assert.Equal(["nested/deep.md", "top.md"], allMarkdown.Select(r => r.FileName).OrderBy(x => x, StringComparer.Ordinal)); + + // And the non-recursive rule still excludes the nested file. + Assert.Equal(["notes.txt", "top.md"], shallow.Select(r => r.FileName).OrderBy(x => x, StringComparer.Ordinal)); + } + + [Fact] + public async Task BaseSearch_NarrowsThroughTheHookAsync() + { + // Arrange: three files match, but only one is indexed. Under-returning breaks the hook's + // contract; it is done here because nothing else proves the hook chose what got read. + var store = new NarrowingStore(); + for (int i = 0; i < 3; i++) + { + await store.WriteAsync($"f{i}.txt", $"alpha\n{Needle}\n"); + } + + store.Indexed.Add("f1.txt"); + + // Act + IReadOnlyList results = await store.SearchAsync(string.Empty, Needle, recursive: true); + + // Assert: narrowing decides what is read; the base still numbers it. + Assert.Equal("f1.txt", Assert.Single(results).FileName); + Assert.Equal(2, results[0].MatchingLines[0].LineNumber); + } + + [Fact] + public async Task ShippedStores_DeclareTheirLineNumbersAlignedAsync() + { + // Assert: both report through ScanContent, so neither is ever re-checked. Asserting only one + // of them would let the other's override be dropped with every test still green, silently + // doubling reads on every grep. + Assert.True(new InMemoryAgentFileStore().ReportsAlignedLineNumbers); + Assert.True(new FileSystemAgentFileStore(Path.GetTempPath()).ReportsAlignedLineNumbers); + Assert.False(new ContentOnlyStore().ReportsAlignedLineNumbers); + + // And a store on the base path is recognised without declaring anything, because the base + // implementation tags the results it numbered. + var store = new ContentOnlyStore(); + await store.WriteAsync("f.txt", Needle); + IReadOnlyList results = await store.SearchAsync(string.Empty, Needle, recursive: true); + await SearchAlignment.ThrowIfMisalignedAsync(store, string.Empty, results, Needle, CancellationToken.None); + } + + [Fact] + public async Task Alignment_RefusesAStoreWhoseLineNumbersAreSkewedAsync() + { + // Arrange: the match is on line 2, but the store reports line 1. + var store = new SkewedStore(); + await store.WriteAsync("cfg.txt", $"alpha\n{Needle}\n"); + IReadOnlyList results = await store.SearchAsync(string.Empty, Needle, recursive: true); + + // Act + Assert + InvalidOperationException error = await Assert.ThrowsAsync( + () => SearchAlignment.ThrowIfMisalignedAsync(store, string.Empty, results, Needle, CancellationToken.None)); + Assert.Contains("do not line up", error.Message, StringComparison.Ordinal); + } + + [Fact] + public async Task Alignment_BelievesAStoreThatDeclaresItsNumbersAlignedAsync() + { + // Arrange: the same wrong numbers, but the store declares alignment. + var store = new LyingStore(); + await store.WriteAsync("cfg.txt", $"alpha\n{Needle}\n"); + IReadOnlyList results = await store.SearchAsync(string.Empty, Needle, recursive: true); + + // Act: no throw. The opt-out is a promise, not a hint. + await SearchAlignment.ThrowIfMisalignedAsync(store, string.Empty, results, Needle, CancellationToken.None); + + // Assert + Assert.Equal(1, results[0].MatchingLines[0].LineNumber); + } + + [Fact] + public async Task Alignment_RefusesBaseResultsRenumberedInPlaceAsync() + { + // Arrange: the store delegates to the base, then edits the numbers without rebuilding the list, + // so the tag it returns is genuinely the base's. + var store = new RenumberingStore(); + await store.WriteAsync("cfg.txt", $"alpha\n{Needle}\n"); + IReadOnlyList results = await store.SearchAsync(string.Empty, Needle, recursive: true); + + // Act + Assert: trust has to come from the snapshot, not from the tag surviving. + InvalidOperationException error = await Assert.ThrowsAsync( + () => SearchAlignment.ThrowIfMisalignedAsync(store, string.Empty, results, Needle, CancellationToken.None)); + Assert.Contains("do not line up", error.Message, StringComparison.Ordinal); + } + + [Fact] + public async Task Alignment_RefusesANonPositiveLineNumberAsync() + { + // Arrange: line 0 is out of range downwards, which an upper-bound check alone lets through. + var store = new ZeroLineStore(); + await store.WriteAsync("cfg.txt", $"alpha\n{Needle}\n"); + IReadOnlyList results = await store.SearchAsync(string.Empty, Needle, recursive: true); + + // Act + Assert: the misalignment error, not a raw index error. + InvalidOperationException error = await Assert.ThrowsAsync( + () => SearchAlignment.ThrowIfMisalignedAsync(store, string.Empty, results, Needle, CancellationToken.None)); + Assert.Contains("do not line up", error.Message, StringComparison.Ordinal); + } + + [Fact] + public async Task Alignment_KeepsCheckingAfterAMatchItCannotEvaluateAsync() + { + // Arrange: trap.txt backtracks past the budget, cfg.txt is skewed and evaluates instantly. + var store = new TrapThenSkewedStore(); + await store.WriteAsync("trap.txt", new string('a', 30)); + await store.WriteAsync("cfg.txt", "zzz\naab\n"); + IReadOnlyList results = await store.SearchAsync(string.Empty, Pathological, recursive: true); + + // Act + Assert: giving up at the trap would let cfg.txt through unchecked. + InvalidOperationException error = await Assert.ThrowsAsync( + () => SearchAlignment.ThrowIfMisalignedAsync( + store, string.Empty, results, Pathological, CancellationToken.None, matchTimeout: TimeSpan.FromMilliseconds(1))); + Assert.Contains("do not line up", error.Message, StringComparison.Ordinal); + } + + [Fact] + public void ApplyReplaceLines_ExpectedLineMatching_AppliesTheEdit() + { + // Act + string result = FileEditor.ApplyReplaceLines( + "one\ntwo\nthree\n", + [new FileLineEdit { LineNumber = 2, NewLine = "TWO\n", ExpectedLine = "two" }]); + + // Assert + Assert.Equal("one\nTWO\nthree\n", result); + } + + [Fact] + public void ApplyReplaceLines_ExpectedLineDiffering_Throws() + { + // Act + Assert: a stale or mis-numbered edit is refused rather than applied. + ArgumentException error = Assert.Throws(() => + FileEditor.ApplyReplaceLines( + "one\ntwo\nthree\n", + [new FileLineEdit { LineNumber = 3, NewLine = "X\n", ExpectedLine = "two" }])); + + Assert.Contains("does not contain the expected text", error.Message, StringComparison.Ordinal); + } + + [Fact] + public void ApplyReplaceLines_ExpectedLineIgnoresTheTerminator() + { + // Act: a line fed straight back from grep still carries its terminator. + string result = FileEditor.ApplyReplaceLines( + "alpha\r\nbeta\r\n", + [new FileLineEdit { LineNumber = 2, NewLine = "BETA\r\n", ExpectedLine = "beta\r\n" }]); + + // Assert + Assert.Equal("alpha\r\nBETA\r\n", result); + } + + [Fact] + public void ApplyReplaceLines_WithoutExpectedLine_IsUnchanged() + { + // Act: the guard is opt-in. + string result = FileEditor.ApplyReplaceLines("one\ntwo\n", [new FileLineEdit { LineNumber = 1, NewLine = "ONE\n" }]); + + // Assert + Assert.Equal("ONE\ntwo\n", result); + } + + /// Overrides SearchAsync but delegates to the base implementation some of the time. + private sealed class SometimesDelegatesStore : ContentOnlyStore + { + public bool Delegate { get; set; } + + public override Task> SearchAsync(string directory, string regexPattern, string? globPattern = null, bool recursive = false, CancellationToken cancellationToken = default) + => this.Delegate + ? base.SearchAsync(directory, regexPattern, globPattern, recursive, cancellationToken) + : Task.FromResult>( + [ + new FileSearchResult + { + FileName = "cfg.txt", + Snippet = string.Empty, + + // Wrong: the match is on line 2. + MatchingLines = [new FileSearchMatch { LineNumber = 1, Line = Needle }], + } + ]); + } + + [Fact] + public async Task Alignment_IsNotDisabledByAnEarlierDelegationToBaseSearchAsync() + { + // Arrange: a store that falls back to the base implementation sometimes -- an index + // warm-up, an unindexed directory. That must not buy permanent trust for the results + // it numbers itself. + var store = new SometimesDelegatesStore(); + await store.WriteAsync("cfg.txt", $"alpha\n{Needle}\n"); + + store.Delegate = true; + _ = await store.SearchAsync(string.Empty, Needle, recursive: true); + + // Act: now it serves its own, skewed numbering. + store.Delegate = false; + IReadOnlyList skewed = await store.SearchAsync(string.Empty, Needle, recursive: true); + + // Assert: still checked, still refused. + await Assert.ThrowsAsync( + () => SearchAlignment.ThrowIfMisalignedAsync(store, string.Empty, skewed, Needle, CancellationToken.None)); + } +} 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 505e761ca9d..99b4f37eae8 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileStore/FileEditorTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileStore/FileEditorTests.cs @@ -2,6 +2,7 @@ using System; using System.Collections.Generic; +using System.Text.RegularExpressions; namespace Microsoft.Agents.AI.UnitTests.Harness.FileMemory; @@ -178,4 +179,130 @@ 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)); + } + + [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 LineContentLength_ExcludesOnlyTheTrailingTerminator(string line, string expected) + { + // Act + int length = FileEditor.LineContentLength(line); + + // Assert — the length delimits exactly the line's text, which is the range searches match over. + Assert.Equal(expected.Length, length); + Assert.Equal(expected, line.Substring(0, length)); + } + + [Theory] + [InlineData("beta match\r\n")] + [InlineData("beta match\n")] + [InlineData("beta match\r")] + [InlineData("beta match")] + public void LineContentLength_BoundsAnEndAnchoredMatch(string line) + { + // Arrange — the callers match over a range instead of a trimmed copy, so '$' has to anchor at + // the returned length rather than at the end of the string. + var regex = new Regex("match$", RegexOptions.IgnoreCase); + + // Act + Match match = regex.Match(line, 0, FileEditor.LineContentLength(line)); + + // Assert + Assert.True(match.Success); + Assert.Equal(5, match.Index); + } + + #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/FileSystemAgentFileStoreTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileStore/FileSystemAgentFileStoreTests.cs index 20d1a8333d6..550c95f8ac2 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileStore/FileSystemAgentFileStoreTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileStore/FileSystemAgentFileStoreTests.cs @@ -293,6 +293,112 @@ public async Task SearchFilesAsync_FindsMatchAsync() Assert.Contains("error", results[0].Snippet); } + // This store carries its own copy of the search loop, so the line, terminator and snippet-offset + // behaviour is pinned here as well as in InMemoryAgentFileStoreTests. A regression in one copy + // would otherwise pass on the strength of the other's coverage. + + [Fact] + public async Task SearchFilesAsync_ReportsLinesVerbatimAsync() + { + // Arrange + await this._store.WriteAsync("notes.md", "Line one\nLine two with match\nLine three\nLine four with match"); + + // Act + var results = await this._store.SearchAsync("", "match"); + + // Assert + Assert.Single(results); + Assert.Equal(2, results[0].MatchingLines.Count); + Assert.Equal(2, results[0].MatchingLines[0].LineNumber); + // 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 SearchFilesAsync_ReportsCrlfLinesVerbatimAsync() + { + // Arrange + await this._store.WriteAsync("notes.md", "alpha\r\nbeta match\r\ngamma\r\n"); + + // Act + var results = await this._store.SearchAsync("", "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 SearchFilesAsync_TrailingNewline_DoesNotReportAnExtraLineAsync() + { + // Arrange — a newline-terminated file has as many lines as the line editor sees, not one more. + await this._store.WriteAsync("notes.md", "a\nb\n"); + + // Act — a pattern that also matches an empty line. + var results = await this._store.SearchAsync("", "^.*$"); + + // 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 SearchFilesAsync_LoneCarriageReturn_SplitsLikeTheLineEditorAsync() + { + // Arrange — a lone '\r' terminates a line for the line editor, so grep must agree. + await this._store.WriteAsync("notes.md", "alpha\rbeta match\rgamma"); + + // Act + var results = await this._store.SearchAsync("", "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); + } + + [Theory] + [InlineData("alpha\r\nbeta match\r\ngamma\r\n")] + [InlineData("alpha\rbeta match\rgamma")] + [InlineData("alpha\nbeta match\ngamma\n")] + public async Task SearchFilesAsync_EndAnchoredPatternMatchesRegardlessOfTerminatorAsync(string content) + { + // Arrange — the pattern anchors to the end of the line's text, which is "beta match". + await this._store.WriteAsync("notes.md", content); + + // Act + var results = await this._store.SearchAsync("", "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 SearchFilesAsync_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. + string padding = new('x', 60); + await this._store.WriteAsync("notes.md", $"{padding}\nneedle\n"); + + // Act + var results = await this._store.SearchAsync("", "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 SearchFilesAsync_GlobFilter_ExcludesNonMatchingAsync() { 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 722dc8f7356..207e8031506 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,100 @@ 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); + } + + [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() + { + // 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() {