diff --git a/NanoAgent.Tests/Application/Tools/FileWriteToolTests.cs b/NanoAgent.Tests/Application/Tools/FileWriteToolTests.cs index 15845556..da71813b 100644 --- a/NanoAgent.Tests/Application/Tools/FileWriteToolTests.cs +++ b/NanoAgent.Tests/Application/Tools/FileWriteToolTests.cs @@ -61,6 +61,40 @@ [new WorkspaceFileEditState("README.md", exists: false, content: null)], transaction!.Description.Should().Be("file_write (README.md)"); } + [Fact] + public async Task ExecuteAsync_Should_PassEmptyContentToWorkspaceService() + { + Mock workspaceFileService = new(MockBehavior.Strict); + workspaceFileService + .Setup(service => service.WriteFileWithTrackingAsync( + ".gitkeep", + string.Empty, + true, + It.IsAny())) + .ReturnsAsync(new WorkspaceFileWriteExecutionResult( + new WorkspaceFileWriteResult( + ".gitkeep", + false, + 0, + 0, + 0, + [], + 0), + new WorkspaceFileEditTransaction( + "file_write (.gitkeep)", + [new WorkspaceFileEditState(".gitkeep", exists: false, content: null)], + [new WorkspaceFileEditState(".gitkeep", exists: true, content: string.Empty)]))); + + FileWriteTool sut = new(workspaceFileService.Object); + + ToolResult result = await sut.ExecuteAsync( + CreateContext("""{ "path": ".gitkeep", "content": "" }"""), + CancellationToken.None); + + result.Status.Should().Be(ToolResultStatus.Success); + workspaceFileService.VerifyAll(); + } + private static ToolExecutionContext CreateContext( string argumentsJson, ReplSessionContext? session = null) diff --git a/NanoAgent.Tests/Infrastructure/Secrets/ProcessRunnerTests.cs b/NanoAgent.Tests/Infrastructure/Secrets/ProcessRunnerTests.cs new file mode 100644 index 00000000..107f8d94 --- /dev/null +++ b/NanoAgent.Tests/Infrastructure/Secrets/ProcessRunnerTests.cs @@ -0,0 +1,39 @@ +using NanoAgent.Infrastructure.Secrets; +using FluentAssertions; + +namespace NanoAgent.Tests.Infrastructure.Secrets; + +public sealed class ProcessRunnerTests +{ + [Fact] + public async Task RunAsync_Should_CapCapturedStandardOutputAndError() + { + ProcessExecutionRequest request = OperatingSystem.IsWindows() + ? new ProcessExecutionRequest( + "powershell", + [ + "-NoProfile", + "-NonInteractive", + "-Command", + "[Console]::Out.Write(('o' * 20000)); [Console]::Error.Write(('e' * 20000))" + ], + MaxOutputCharacters: 128) + : new ProcessExecutionRequest( + "/bin/sh", + [ + "-c", + "printf '%*s' 20000 '' | tr ' ' o; printf '%*s' 20000 '' | tr ' ' e >&2" + ], + MaxOutputCharacters: 128); + + ProcessExecutionResult result = await new ProcessRunner().RunAsync( + request, + CancellationToken.None); + + result.ExitCode.Should().Be(0); + result.StandardOutput.Length.Should().BeLessThanOrEqualTo(128); + result.StandardError.Length.Should().BeLessThanOrEqualTo(128); + result.StandardOutput.Should().EndWith("..."); + result.StandardError.Should().EndWith("..."); + } +} diff --git a/NanoAgent.Tests/Infrastructure/Tools/ShellCommandServiceTests.cs b/NanoAgent.Tests/Infrastructure/Tools/ShellCommandServiceTests.cs index 23925014..e970dc46 100644 --- a/NanoAgent.Tests/Infrastructure/Tools/ShellCommandServiceTests.cs +++ b/NanoAgent.Tests/Infrastructure/Tools/ShellCommandServiceTests.cs @@ -42,6 +42,7 @@ await sut.ExecuteAsync( processRunner.Requests[0].FileName.Should().Be("/bin/bash"); processRunner.Requests[0].Arguments.Should().Equal("-lc", "node -v && npm -v"); processRunner.Requests[0].WorkingDirectory.Should().Be(Path.Combine(_workspaceRoot, "src")); + processRunner.Requests[0].MaxOutputCharacters.Should().Be(8000); } [Fact] @@ -65,6 +66,7 @@ await sut.ExecuteAsync( processRunner.Requests.Should().ContainSingle(); ProcessExecutionRequest request = processRunner.Requests[0]; request.FileName.Should().Be("powershell"); + request.MaxOutputCharacters.Should().Be(8000); request.Arguments.Should().Contain("-Command"); request.Arguments[^1].Should().Contain("Invoke-NanoSegment"); request.Arguments[^1].Should().Contain("FromBase64String"); diff --git a/NanoAgent.Tests/Infrastructure/Tools/WorkspaceFileServiceTests.cs b/NanoAgent.Tests/Infrastructure/Tools/WorkspaceFileServiceTests.cs index 6116cc44..27d3754c 100644 --- a/NanoAgent.Tests/Infrastructure/Tools/WorkspaceFileServiceTests.cs +++ b/NanoAgent.Tests/Infrastructure/Tools/WorkspaceFileServiceTests.cs @@ -65,6 +65,61 @@ await File.WriteAllTextAsync( new WorkspaceFileWritePreviewLine(3, "context", "}")); } + [Fact] + public async Task WriteFileAsync_Should_AllowEmptyContent() + { + WorkspaceFileService sut = CreateSut(); + + WorkspaceFileWriteResult result = await sut.WriteFileAsync( + ".gitkeep", + string.Empty, + overwrite: true, + CancellationToken.None); + + result.CharacterCount.Should().Be(0); + result.AddedLineCount.Should().Be(0); + result.PreviewLines.Should().BeEmpty(); + File.ReadAllText(Path.Combine(_workspaceRoot, ".gitkeep")) + .Should() + .BeEmpty(); + } + + [Fact] + public async Task WriteFileAsync_Should_TruncateExistingFileToEmptyContent() + { + WorkspaceFileService sut = CreateSut(); + string filePath = Path.Combine(_workspaceRoot, "settings.json"); + await File.WriteAllTextAsync(filePath, "{}\n", CancellationToken.None); + + WorkspaceFileWriteResult result = await sut.WriteFileAsync( + "settings.json", + string.Empty, + overwrite: true, + CancellationToken.None); + + result.OverwroteExistingFile.Should().BeTrue(); + result.CharacterCount.Should().Be(0); + File.ReadAllText(filePath).Should().BeEmpty(); + } + + [Fact] + public async Task WriteFileAsync_Should_WriteUtf8WithoutBom() + { + WorkspaceFileService sut = CreateSut(); + + await sut.WriteFileAsync( + "script.sh", + "#!/bin/sh\necho hi\n", + overwrite: true, + CancellationToken.None); + + byte[] bytes = await File.ReadAllBytesAsync( + Path.Combine(_workspaceRoot, "script.sh"), + CancellationToken.None); + + bytes.Take(3).Should().NotEqual(new byte[] { 0xEF, 0xBB, 0xBF }); + } + [Fact] public async Task ReadFileAsync_Should_ReadFileContent() { @@ -149,6 +204,54 @@ class Program .Should().Be("remember the tests"); } + [Fact] + public async Task ApplyPatchAsync_Should_AddFinalNewline_When_RemovedLineHadNoNewlineMarker() + { + WorkspaceFileService sut = CreateSut(); + string filePath = Path.Combine(_workspaceRoot, "settings.json"); + await File.WriteAllTextAsync(filePath, "{}", CancellationToken.None); + + await sut.ApplyPatchAsync( + """ + *** Begin Patch + *** Update File: settings.json + @@ + -{} + \ No newline at end of file + +{} + *** End Patch + """, + CancellationToken.None); + + (await File.ReadAllTextAsync(filePath, CancellationToken.None)) + .Should() + .Be("{}\n"); + } + + [Fact] + public async Task ApplyPatchAsync_Should_RemoveFinalNewline_When_AddedLineHasNoNewlineMarker() + { + WorkspaceFileService sut = CreateSut(); + string filePath = Path.Combine(_workspaceRoot, "settings.json"); + await File.WriteAllTextAsync(filePath, "{}\n", CancellationToken.None); + + await sut.ApplyPatchAsync( + """ + *** Begin Patch + *** Update File: settings.json + @@ + -{} + +{} + \ No newline at end of file + *** End Patch + """, + CancellationToken.None); + + (await File.ReadAllTextAsync(filePath, CancellationToken.None)) + .Should() + .Be("{}"); + } + [Fact] public async Task WriteFileWithTrackingAsync_Should_ReturnUndoableBeforeAndAfterStates() { diff --git a/NanoAgent/Infrastructure/Secrets/ProcessExecutionRequest.cs b/NanoAgent/Infrastructure/Secrets/ProcessExecutionRequest.cs index 49498c94..92f98512 100644 --- a/NanoAgent/Infrastructure/Secrets/ProcessExecutionRequest.cs +++ b/NanoAgent/Infrastructure/Secrets/ProcessExecutionRequest.cs @@ -4,4 +4,5 @@ internal sealed record ProcessExecutionRequest( string FileName, IReadOnlyList Arguments, string? StandardInput = null, - string? WorkingDirectory = null); + string? WorkingDirectory = null, + int? MaxOutputCharacters = null); diff --git a/NanoAgent/Infrastructure/Secrets/ProcessRunner.cs b/NanoAgent/Infrastructure/Secrets/ProcessRunner.cs index 615f21b4..b43bddf3 100644 --- a/NanoAgent/Infrastructure/Secrets/ProcessRunner.cs +++ b/NanoAgent/Infrastructure/Secrets/ProcessRunner.cs @@ -1,4 +1,5 @@ using System.Diagnostics; +using System.Text; namespace NanoAgent.Infrastructure.Secrets; @@ -37,8 +38,14 @@ public async Task RunAsync( process.Start(); - Task standardOutputTask = process.StandardOutput.ReadToEndAsync(cancellationToken); - Task standardErrorTask = process.StandardError.ReadToEndAsync(cancellationToken); + Task standardOutputTask = ReadToEndCappedAsync( + process.StandardOutput, + request.MaxOutputCharacters, + cancellationToken); + Task standardErrorTask = ReadToEndCappedAsync( + process.StandardError, + request.MaxOutputCharacters, + cancellationToken); if (request.StandardInput is not null) { @@ -76,4 +83,70 @@ private static void TryKillProcess(Process process) { } } + + private static async Task ReadToEndCappedAsync( + TextReader reader, + int? maxCharacters, + CancellationToken cancellationToken) + { + const int BufferSize = 4096; + + if (maxCharacters is <= 0) + { + await DrainAsync(reader, cancellationToken); + return string.Empty; + } + + char[] buffer = new char[BufferSize]; + StringBuilder builder = maxCharacters is null + ? new StringBuilder() + : new StringBuilder(Math.Min(maxCharacters.Value, BufferSize)); + bool truncated = false; + + while (true) + { + int read = await reader.ReadAsync( + buffer.AsMemory(0, buffer.Length), + cancellationToken); + if (read == 0) + { + break; + } + + if (maxCharacters is null) + { + builder.Append(buffer, 0, read); + continue; + } + + int remaining = maxCharacters.Value - builder.Length; + if (remaining <= 0) + { + truncated = true; + continue; + } + + int charactersToAppend = Math.Min(read, remaining); + builder.Append(buffer, 0, charactersToAppend); + truncated |= charactersToAppend < read; + } + + if (truncated && maxCharacters is > 3) + { + builder.Length = Math.Min(builder.Length, maxCharacters.Value - 3); + builder.Append("..."); + } + + return builder.ToString(); + } + + private static async Task DrainAsync( + TextReader reader, + CancellationToken cancellationToken) + { + char[] buffer = new char[4096]; + while (await reader.ReadAsync(buffer.AsMemory(0, buffer.Length), cancellationToken) > 0) + { + } + } } diff --git a/NanoAgent/Infrastructure/Tools/ShellCommandService.cs b/NanoAgent/Infrastructure/Tools/ShellCommandService.cs index 5be56a72..be44fa8e 100644 --- a/NanoAgent/Infrastructure/Tools/ShellCommandService.cs +++ b/NanoAgent/Infrastructure/Tools/ShellCommandService.cs @@ -43,11 +43,13 @@ public async Task ExecuteAsync( ? new ProcessExecutionRequest( "powershell", ["-NoProfile", "-NonInteractive", "-Command", commandText], - WorkingDirectory: workingDirectory) + WorkingDirectory: workingDirectory, + MaxOutputCharacters: MaxOutputCharacters) : new ProcessExecutionRequest( "/bin/bash", ["-lc", request.Command], - WorkingDirectory: workingDirectory); + WorkingDirectory: workingDirectory, + MaxOutputCharacters: MaxOutputCharacters); ProcessExecutionResult result = await _processRunner.RunAsync( processRequest, diff --git a/NanoAgent/Infrastructure/Tools/WorkspaceFileService.cs b/NanoAgent/Infrastructure/Tools/WorkspaceFileService.cs index c1a6bae0..8a70f0e4 100644 --- a/NanoAgent/Infrastructure/Tools/WorkspaceFileService.cs +++ b/NanoAgent/Infrastructure/Tools/WorkspaceFileService.cs @@ -17,6 +17,8 @@ internal sealed class WorkspaceFileService : IWorkspaceFileService private const int FileWritePreviewContextLines = 1; private const int MaxFileWritePreviewLines = 8; + private static readonly Encoding Utf8NoBom = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false); + private readonly IWorkspaceRootProvider _workspaceRootProvider; public WorkspaceFileService(IWorkspaceRootProvider workspaceRootProvider) @@ -90,7 +92,7 @@ public async Task ApplyFileEditStatesAsync( await File.WriteAllTextAsync( fullPath, state.Content!, - Encoding.UTF8, + Utf8NoBom, cancellationToken); } @@ -241,12 +243,7 @@ public async Task WriteFileAsync( CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); - - if (string.IsNullOrWhiteSpace(content)) - { - throw new InvalidOperationException( - "File content must not be empty."); - } + ArgumentNullException.ThrowIfNull(content); string fullPath = ResolveWorkspacePath(path, directoryRequired: false, fileRequired: false); bool fileExists = File.Exists(fullPath); @@ -270,7 +267,7 @@ public async Task WriteFileAsync( await File.WriteAllTextAsync( fullPath, content, - Encoding.UTF8, + Utf8NoBom, cancellationToken); FileWritePreview preview = BuildFileWritePreview(previousContent, content); @@ -346,7 +343,7 @@ private async Task ApplyAddFileOperationAsync( await File.WriteAllTextAsync( fullPath, content, - Encoding.UTF8, + Utf8NoBom, cancellationToken); return CreatePatchFileResult( @@ -404,7 +401,7 @@ private async Task ApplyUpdateFileOperationAsync( await File.WriteAllTextAsync( destinationFullPath, updatedContent, - Encoding.UTF8, + Utf8NoBom, cancellationToken); if (!string.Equals(currentFullPath, destinationFullPath, GetPathComparison()) && @@ -446,9 +443,7 @@ private WorkspaceDirectoryEntry[] ListDirectoryManaged( string fullPath, bool recursive) { - IEnumerable entries = recursive - ? Directory.EnumerateFileSystemEntries(fullPath, "*", SearchOption.AllDirectories) - : Directory.EnumerateFileSystemEntries(fullPath, "*", SearchOption.TopDirectoryOnly); + IEnumerable entries = EnumerateFileSystemEntriesSafely(fullPath, recursive); return entries .OrderBy(static entry => entry, StringComparer.Ordinal) @@ -469,7 +464,10 @@ private IReadOnlyList SearchFilesManaged( IEnumerable files = File.Exists(fullPath) ? [fullPath] - : Directory.EnumerateFiles(fullPath, "*", SearchOption.AllDirectories); + : Directory.Exists(fullPath) + ? EnumerateFilesSafely(fullPath, recursive: true) + : throw new FileNotFoundException( + $"Search path '{request.Path ?? "."}' does not exist."); return files .OrderBy(static path => path, StringComparer.Ordinal) @@ -492,10 +490,7 @@ private async Task> SearchTextManagedAsy } else if (Directory.Exists(fullPath)) { - filesToSearch.AddRange(Directory.EnumerateFiles( - fullPath, - "*", - SearchOption.AllDirectories)); + filesToSearch.AddRange(EnumerateFilesSafely(fullPath, recursive: true)); } else { @@ -512,8 +507,8 @@ private async Task> SearchTextManagedAsy { cancellationToken.ThrowIfCancellationRequested(); - FileInfo fileInfo = new(filePath); - if (fileInfo.Length > MaxSearchFileBytes) + if (!TryGetFileLength(filePath, out long fileLength) || + fileLength > MaxSearchFileBytes) { continue; } @@ -534,6 +529,10 @@ private async Task> SearchTextManagedAsy { continue; } + catch (Exception exception) when (IsFileSystemAccessException(exception)) + { + continue; + } for (int index = 0; index < lines.Length; index++) { @@ -559,6 +558,89 @@ private async Task> SearchTextManagedAsy return matches; } + private static IEnumerable EnumerateFilesSafely( + string root, + bool recursive) + { + return EnumerateFileSystemEntriesSafely(root, recursive) + .Where(static entry => File.Exists(entry)); + } + + private static IEnumerable EnumerateFileSystemEntriesSafely( + string root, + bool recursive) + { + Stack pendingDirectories = new(); + pendingDirectories.Push(root); + + while (pendingDirectories.Count > 0) + { + string directoryPath = pendingDirectories.Pop(); + string[] entries; + try + { + entries = Directory.GetFileSystemEntries(directoryPath); + } + catch (Exception exception) when (IsFileSystemAccessException(exception)) + { + continue; + } + + foreach (string entry in entries) + { + yield return entry; + + if (recursive && ShouldRecurseIntoDirectory(entry)) + { + pendingDirectories.Push(entry); + } + } + + if (!recursive) + { + yield break; + } + } + } + + private static bool TryGetFileLength( + string path, + out long length) + { + try + { + length = new FileInfo(path).Length; + return true; + } + catch (Exception exception) when (IsFileSystemAccessException(exception)) + { + length = 0; + return false; + } + } + + private static bool ShouldRecurseIntoDirectory(string path) + { + try + { + FileAttributes attributes = File.GetAttributes(path); + return attributes.HasFlag(FileAttributes.Directory) && + !attributes.HasFlag(FileAttributes.ReparsePoint); + } + catch (Exception exception) when (IsFileSystemAccessException(exception)) + { + return false; + } + } + + private static bool IsFileSystemAccessException(Exception exception) + { + return exception is UnauthorizedAccessException or + IOException or + PathTooLongException or + System.Security.SecurityException; + } + private string ResolveWorkspacePath( string? requestedPath, bool directoryRequired, @@ -780,8 +862,20 @@ private static PatchOperation ParseUpdateFile( !lines[lineIndex].StartsWith("*** ", StringComparison.Ordinal)) { string line = lines[lineIndex]; - if (string.Equals(line, "\\ No newline at end of file", StringComparison.Ordinal) || - string.Equals(line, "*** End of File", StringComparison.Ordinal)) + if (string.Equals(line, "\\ No newline at end of file", StringComparison.Ordinal)) + { + if (currentHunkLines is null || currentHunkLines.Count == 0) + { + throw new FormatException("No-newline patch markers must follow a patch line."); + } + + PatchLine previousLine = currentHunkLines[^1]; + currentHunkLines[^1] = previousLine with { NoNewlineAtEnd = true }; + lineIndex++; + continue; + } + + if (string.Equals(line, "*** End of File", StringComparison.Ordinal)) { lineIndex++; continue; @@ -813,7 +907,8 @@ private static PatchOperation ParseUpdateFile( '-' => PatchLineKind.Removal, _ => throw new FormatException($"Invalid patch line prefix in '{line}'.") }, - line[1..])); + line[1..], + NoNewlineAtEnd: false)); lineIndex++; } @@ -867,6 +962,7 @@ private static string ApplyUpdatePatch( { List currentLines = SplitLines(previousContent).ToList(); int searchStart = 0; + bool? trailingNewLineOverride = null; foreach (PatchHunk hunk in hunks) { @@ -897,12 +993,33 @@ private static string ApplyUpdatePatch( currentLines.RemoveRange(matchIndex, beforeLines.Length); currentLines.InsertRange(matchIndex, afterLines); searchStart = matchIndex + afterLines.Length; + trailingNewLineOverride = GetTrailingNewLineOverride(hunk) ?? trailingNewLineOverride; } - bool trailingNewLine = previousContent.EndsWith('\n') || previousContent.EndsWith('\r'); + bool trailingNewLine = trailingNewLineOverride ?? + (previousContent.EndsWith('\n') || previousContent.EndsWith('\r')); return JoinLines(currentLines, trailingNewLine); } + private static bool? GetTrailingNewLineOverride(PatchHunk hunk) + { + bool? trailingNewLine = null; + + foreach (PatchLine line in hunk.Lines) + { + if (!line.NoNewlineAtEnd) + { + continue; + } + + trailingNewLine = line.Kind is PatchLineKind.Addition or PatchLineKind.Context + ? false + : true; + } + + return trailingNewLine; + } + private static int FindSequence( IReadOnlyList source, IReadOnlyList target, @@ -1120,7 +1237,8 @@ private readonly record struct PatchHunk( private readonly record struct PatchLine( PatchLineKind Kind, - string Text); + string Text, + bool NoNewlineAtEnd); private enum PatchOperationKind {