diff --git a/Directory.Build.props b/Directory.Build.props index 79938000..2bd19673 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -9,7 +9,7 @@ -0.14.10 +0.14.11 diff --git a/RockBot.slnx b/RockBot.slnx index 55c6b061..ea9335df 100644 --- a/RockBot.slnx +++ b/RockBot.slnx @@ -59,6 +59,7 @@ + diff --git a/deploy/helm/rockbot/templates/_helpers.tpl b/deploy/helm/rockbot/templates/_helpers.tpl index b7131a1b..2ea1e9dd 100644 --- a/deploy/helm/rockbot/templates/_helpers.tpl +++ b/deploy/helm/rockbot/templates/_helpers.tpl @@ -72,3 +72,24 @@ Name of the shared volume PVC. {{- define "rockbot.sharedPvcName" -}} {{- include "rockbot.fullname" . }}-shared {{- end }} + +{{/* +find(1) exclusion clauses for shared.protectedPaths, one pair of lines per entry. + +Each entry is stripped of surrounding slashes first: a trailing one would render +'.../notes//*', which fnmatch cannot match against '.../notes/x.md', silently +disabling the protection the operator asked for. Two clauses per entry so a +protected leaf file is covered as well as a directory's contents. + +Emitted without indentation — callers apply `nindent` and must guard on the result +being empty, since a blank continuation line would break the shell command. +*/}} +{{- define "rockbot.sharedProtectedFindClauses" -}} +{{- range .Values.shared.protectedPaths }} +{{- $prefix := . | trimPrefix "/" | trimSuffix "/" }} +{{- if $prefix }} +! -path '/rockbot/shared/{{ $prefix }}' \ +! -path '/rockbot/shared/{{ $prefix }}/*' \ +{{- end }} +{{- end }} +{{- end }} diff --git a/deploy/helm/rockbot/templates/shared/cronjob.yaml b/deploy/helm/rockbot/templates/shared/cronjob.yaml index 71dcd5c5..0a43bafc 100644 --- a/deploy/helm/rockbot/templates/shared/cronjob.yaml +++ b/deploy/helm/rockbot/templates/shared/cronjob.yaml @@ -25,13 +25,30 @@ spec: - sh - -c - | + {{- $protected := include "rockbot.sharedProtectedFindClauses" . | trim }} mkdir -p /rockbot/shared/tmp /rockbot/shared/drafts /rockbot/shared/exports - find /rockbot/shared/tmp -mindepth 1 -mtime +{{ .Values.shared.tmpTtlDays }} -delete - find /rockbot/shared/drafts -mindepth 1 -mtime +{{ .Values.shared.draftsTtlDays }} -delete - find /rockbot/shared/exports -mindepth 1 -mtime +{{ .Values.shared.exportsTtlDays }} -delete + # Per-directory sweeps. Prefixes in shared.protectedPaths are exempt + # from these too: the setting reads as a global promise, and an operator + # who protects a path that happens to sit under drafts/ would otherwise + # get no protection at all with nothing to indicate why. + # `!` rather than `-not`: busybox find does not reliably provide the + # GNU spelling. + find /rockbot/shared/tmp -mindepth 1 -mtime +{{ .Values.shared.tmpTtlDays }} \ + {{- if $protected }}{{ $protected | nindent 20 }}{{ end }} + -delete + find /rockbot/shared/drafts -mindepth 1 -mtime +{{ .Values.shared.draftsTtlDays }} \ + {{- if $protected }}{{ $protected | nindent 20 }}{{ end }} + -delete + find /rockbot/shared/exports -mindepth 1 -mtime +{{ .Values.shared.exportsTtlDays }} \ + {{- if $protected }}{{ $protected | nindent 20 }}{{ end }} + -delete # Catch-all: any remaining file anywhere on the shared volume older than # globalTtlDays. Uses -type f so directory structure is preserved. - find /rockbot/shared -mindepth 1 -type f -mtime +{{ .Values.shared.globalTtlDays }} -delete + # This sweep keys on mtime, so durable content that is legitimately not + # edited for a long time is exactly what it would delete. + find /rockbot/shared -mindepth 1 -type f -mtime +{{ .Values.shared.globalTtlDays }} \ + {{- if $protected }}{{ $protected | nindent 20 }}{{ end }} + -delete volumeMounts: - name: shared-data mountPath: /rockbot/shared diff --git a/deploy/helm/rockbot/values.yaml b/deploy/helm/rockbot/values.yaml index 4461f063..3bb1c782 100644 --- a/deploy/helm/rockbot/values.yaml +++ b/deploy/helm/rockbot/values.yaml @@ -256,6 +256,19 @@ shared: # not touched, so empty structure is preserved. Safety net for ad-hoc paths # (e.g. patrol drafts, subagent artifacts) that don't have a dedicated TTL. globalTtlDays: 30 + # Path prefixes (relative to the volume root) exempt from the catch-all sweep. + # The sweep keys on mtime, so a file is deleted for not having changed recently — + # which is the normal state of durable content such as reference corpora, campaign + # canon, or any document set the agent reads far more often than it edits. Anything + # that must survive longer than globalTtlDays without an edit belongs here. + # Matching is a glob on the full path, so nested content (including a `.git` + # directory) under a listed prefix is protected too. A single file may be listed + # as well as a directory. Leading and trailing slashes are stripped, so `canon`, + # `/canon`, and `canon/` are equivalent. Entries are exempt from every sweep, + # the per-directory ones above included. + # protectedPaths: + # - canon + protectedPaths: [] # Shared POSIX group ID applied via pod-level securityContext.fsGroup on every # pod that mounts the rockbot-shared PVC (agent, shared-cleanup cronjob, # ephemeral script pods). kubelet chgrp's the volume root to this GID, sets diff --git a/src/RockBot.Host/TextEdit.cs b/src/RockBot.Host/TextEdit.cs new file mode 100644 index 00000000..aadbcd4d --- /dev/null +++ b/src/RockBot.Host/TextEdit.cs @@ -0,0 +1,254 @@ +namespace RockBot.Host; + +/// +/// Outcome classification for an call. +/// +public enum TextEditStatus +{ + /// The edit was applied. + Success, + + /// oldText does not occur in the content. + NotFound, + + /// + /// oldText occurs more than once and replaceAll was not set. + /// The caller must supply more surrounding context to disambiguate. + /// + Ambiguous, + + /// oldText was empty — an empty match has no well-defined location. + EmptyOldText, + + /// oldText and newText are identical, so the edit is a no-op. + NoChange, +} + +/// +/// Result of an exact-match text edit. +/// +/// Outcome classification. +/// +/// The edited content when is ; +/// null otherwise. +/// +/// Number of occurrences replaced. Zero unless successful. +/// Human-readable failure description; null on success. +public readonly record struct TextEditResult( + TextEditStatus Status, + string? Content, + int ReplacementCount, + string? Error) +{ + /// Whether the edit succeeded. + public bool IsSuccess => Status == TextEditStatus.Success; +} + +/// +/// Exact-match text replacement — the shared primitive behind surgical edits to +/// files, memory entries, and skill bodies. +/// +/// +/// +/// Every RockBot write surface historically replaced its entire payload: a one-word +/// correction to a document meant re-emitting the whole document, and anything the +/// model failed to reproduce was silently lost. This primitive exists so a caller can +/// state the change instead of restating the content. +/// +/// +/// Matching is ordinal and exact. Ambiguity is an error rather than a guess: when +/// oldText occurs more than once the caller must either widen the match with +/// surrounding context or opt in to replaceAll. That refusal is the point — +/// a tool that silently edits the first of several matches is worse than one that +/// declines, because the caller cannot tell which one it hit. +/// +/// +public static class TextEdit +{ + /// + /// Replaces with in + /// . + /// + /// The content to edit. + /// Exact text to find. Must be non-empty. + /// Replacement text. May be empty to delete. + /// + /// When true, replaces every occurrence. When false (default), more + /// than one occurrence is an error. + /// + /// A describing the outcome. + /// + /// + /// When does not match as supplied, the match is retried + /// with its line endings converted — bare LFs to CRLF, then CRLFs to bare LF. This + /// lets a caller edit a document without having to know its line-ending style, in + /// either direction. + /// + /// + /// is converted to the line-ending style + /// already uses — on the exact-match path as well as the + /// retry path — so an edit cannot leave a single-style document with mixed endings. + /// Content that is already mixed is left alone, having no style to preserve. + /// + /// + public static TextEditResult Apply( + string content, + string oldText, + string newText, + bool replaceAll = false) + { + ArgumentNullException.ThrowIfNull(content); + ArgumentNullException.ThrowIfNull(oldText); + ArgumentNullException.ThrowIfNull(newText); + + if (oldText.Length == 0) + { + return new TextEditResult( + TextEditStatus.EmptyOldText, + null, + 0, + "oldText must not be empty — an empty match has no well-defined location. " + + "To append content, include the trailing text you want to insert before."); + } + + if (string.Equals(oldText, newText, StringComparison.Ordinal)) + { + return new TextEditResult( + TextEditStatus.NoChange, + null, + 0, + "oldText and newText are identical — the edit would change nothing."); + } + + var effectiveOld = oldText; + var effectiveNew = MatchNewlineStyle(newText, content); + var count = CountOccurrences(content, effectiveOld); + + // The caller's line endings do not match the file's. Retry with each conversion + // in turn — LF-supplied text against a CRLF file, and CRLF-supplied text against + // an LF file — so line-ending style is not something the caller has to discover + // by trial and error. + if (count == 0) + { + (string Old, string New)[] candidates = + [ + (ToCrLf(oldText), ToCrLf(newText)), + (ToLf(oldText), ToLf(newText)), + ]; + + foreach (var candidate in candidates) + { + if (string.Equals(candidate.Old, oldText, StringComparison.Ordinal)) + continue; + + var candidateCount = CountOccurrences(content, candidate.Old); + if (candidateCount == 0) + continue; + + effectiveOld = candidate.Old; + effectiveNew = candidate.New; + count = candidateCount; + break; + } + } + + if (count == 0) + { + return new TextEditResult( + TextEditStatus.NotFound, + null, + 0, + "oldText was not found. It must match the content exactly, including " + + "whitespace and indentation. Read the current content and copy the text verbatim."); + } + + // Newline normalization can collapse a difference that the raw arguments had — + // "a\r\nb" replaced by "a\nb" in a CRLF file asks for no change at all. + if (string.Equals(effectiveOld, effectiveNew, StringComparison.Ordinal)) + { + return new TextEditResult( + TextEditStatus.NoChange, + null, + 0, + "oldText and newText differ only in line endings, which are normalized to " + + "the style the content already uses — the edit would change nothing."); + } + + if (count > 1 && !replaceAll) + { + return new TextEditResult( + TextEditStatus.Ambiguous, + null, + 0, + $"oldText occurs {count} times — the edit target is ambiguous. Either include " + + "more surrounding text so the match is unique, or set replaceAll to change every occurrence."); + } + + var edited = replaceAll + ? content.Replace(effectiveOld, effectiveNew, StringComparison.Ordinal) + : ReplaceFirst(content, effectiveOld, effectiveNew); + + return new TextEditResult(TextEditStatus.Success, edited, replaceAll ? count : 1, null); + } + + /// + /// Counts non-overlapping ordinal occurrences of . + /// + private static int CountOccurrences(string haystack, string needle) + { + var count = 0; + var index = 0; + + while ((index = haystack.IndexOf(needle, index, StringComparison.Ordinal)) >= 0) + { + count++; + index += needle.Length; + } + + return count; + } + + private static string ReplaceFirst(string content, string oldText, string newText) + { + var index = content.IndexOf(oldText, StringComparison.Ordinal); + return index < 0 + ? content + : string.Concat(content.AsSpan(0, index), newText, content.AsSpan(index + oldText.Length)); + } + + /// + /// Converts to the line-ending style + /// uses, or returns it unchanged when the content has no + /// single style to preserve. + /// + private static string MatchNewlineStyle(string value, string content) + { + if (!value.Contains('\n', StringComparison.Ordinal)) + return value; + + var crLf = CountOccurrences(content, "\r\n"); + var bareLf = CountOccurrences(content, "\n") - crLf; + + if (crLf > 0 && bareLf == 0) + return ToCrLf(value); + + if (bareLf > 0 && crLf == 0) + return ToLf(value); + + // Mixed endings, or none at all — no style to conform to. + return value; + } + + /// + /// Converts bare LF line endings to CRLF, leaving existing CRLF pairs intact. + /// + private static string ToCrLf(string value) => + value.Replace("\r\n", "\n", StringComparison.Ordinal) + .Replace("\n", "\r\n", StringComparison.Ordinal); + + /// + /// Converts CRLF line endings to bare LF. + /// + private static string ToLf(string value) => + value.Replace("\r\n", "\n", StringComparison.Ordinal); +} diff --git a/src/RockBot.Tools.FileSystem/FileEditToolExecutor.cs b/src/RockBot.Tools.FileSystem/FileEditToolExecutor.cs new file mode 100644 index 00000000..b560da2d --- /dev/null +++ b/src/RockBot.Tools.FileSystem/FileEditToolExecutor.cs @@ -0,0 +1,191 @@ +using System.Collections.Concurrent; +using System.Text.Json; +using RockBot.Host; + +namespace RockBot.Tools.FileSystem; + +/// +/// Applies an exact-match replacement to a single file on the shared volume, +/// leaving the rest of the file byte-for-byte untouched. +/// +internal sealed class FileEditToolExecutor(FileSystemOptions options) : IToolExecutor +{ + /// + /// One lock per resolved path, so concurrent edits to the same file serialize while + /// edits to different files do not. Entries are never evicted — one small object per + /// distinct file edited in the process lifetime, bounded by the volume's contents. + /// + private static readonly ConcurrentDictionary PathLocks = new(StringComparer.Ordinal); + + + public async Task ExecuteAsync(ToolInvokeRequest request, CancellationToken ct) + { + try + { + var args = ParseArguments(request.Arguments); + + if (!args.TryGetValue("path", out var pathElement)) + return Error(request, "Missing required argument: path"); + if (!args.TryGetValue("old_string", out var oldElement)) + return Error(request, "Missing required argument: old_string"); + if (!args.TryGetValue("new_string", out var newElement)) + return Error(request, "Missing required argument: new_string"); + + // Each argument must actually be a string. Coalescing a JSON null to "" + // would turn a malformed call into a silent deletion of the matched text. + if (pathElement.ValueKind != JsonValueKind.String) + return Error(request, "Invalid argument: path must be a string."); + if (oldElement.ValueKind != JsonValueKind.String) + return Error(request, "Invalid argument: old_string must be a string."); + if (newElement.ValueKind != JsonValueKind.String) + { + return Error(request, + "Invalid argument: new_string must be a string. To delete the matched " + + "text, pass an empty string."); + } + + var relativePath = pathElement.GetString()!; + var oldString = oldElement.GetString()!; + var newString = newElement.GetString()!; + + if (!TryReadReplaceAll(args, out var replaceAll)) + { + return Error(request, + "Invalid argument: replace_all must be true or false. Silently ignoring " + + "it would refuse your edit as ambiguous with no way to see why."); + } + + var fullPath = FileWriteToolExecutor.SafeResolvePath(options.BasePath, relativePath); + if (fullPath is null) + return Error(request, "Invalid path: must be within the shared volume."); + + if (!File.Exists(fullPath)) + return Error(request, $"File not found: {relativePath}. Use file_write to create it."); + + // Serialize edits to the same file: several subagents can be in flight at + // once, and without this both would read the pre-edit content and the + // second write would erase the first, each reporting success. + var gate = PathLocks.GetOrAdd(fullPath, _ => new SemaphoreSlim(1, 1)); + await gate.WaitAsync(ct); + + try + { + var read = await FileText.ReadAsync(fullPath, ct); + if (!read.IsSuccess) + return Error(request, $"Cannot edit {relativePath}: {read.Error}"); + + var original = read.Content!; + var result = TextEdit.Apply(original, oldString, newString, replaceAll); + + if (!result.IsSuccess) + return Error(request, $"Edit failed on {relativePath}: {result.Error}"); + + // The atomic write replaces the directory entry, which a writable directory + // permits even when the file itself is not writable. Probe first so editing + // keeps the same permission boundary an in-place write would have had. + if (!CanWrite(fullPath)) + { + return Error(request, + $"Permission denied: {relativePath} is not writable. It was created by " + + "another user on the shared volume; ask an operator to fix its ownership " + + "or mode, or write your change to a new file."); + } + + var written = await FileText.WriteAtomicIfUnchangedAsync( + fullPath, read.Bytes!, result.Content!, read.Encoding!, ct); + + if (!written) + { + return Error(request, + $"{relativePath} was modified by something else while this edit was " + + "being prepared, so the edit was not applied — writing it would have " + + "discarded that change. Read the file again and redo the edit."); + } + + var plural = result.ReplacementCount == 1 ? "occurrence" : "occurrences"; + return new ToolInvokeResponse + { + ToolCallId = request.ToolCallId, + ToolName = request.ToolName, + Content = $"Replaced {result.ReplacementCount} {plural} in {relativePath} " + + $"({original.Length} → {result.Content!.Length} characters).", + IsError = false + }; + } + finally + { + gate.Release(); + } + } + catch (Exception ex) + { + return Error(request, $"Edit failed: {ex.Message}"); + } + } + + /// + /// Reads the optional replace_all flag, accepting a JSON boolean or its + /// string spelling. + /// + /// + /// The text-based tool-calling path has no schema to coerce types, so a model may + /// emit "true" rather than true. Treating that as false would + /// refuse the edit as ambiguous while the caller can see it did pass the flag, so + /// the string form is accepted and anything else is an explicit error. + /// + private static bool TryReadReplaceAll(Dictionary args, out bool replaceAll) + { + replaceAll = false; + + if (!args.TryGetValue("replace_all", out var element)) + return true; + + switch (element.ValueKind) + { + case JsonValueKind.True: + replaceAll = true; + return true; + case JsonValueKind.False: + case JsonValueKind.Null: + case JsonValueKind.Undefined: + return true; + case JsonValueKind.String when bool.TryParse(element.GetString(), out var parsed): + replaceAll = parsed; + return true; + default: + return false; + } + } + + /// + /// Whether the file itself can be opened for writing, independent of its directory. + /// + private static bool CanWrite(string path) + { + try + { + using var probe = new FileStream(path, FileMode.Open, FileAccess.Write, FileShare.ReadWrite); + return true; + } + catch (UnauthorizedAccessException) + { + return false; + } + } + + private static ToolInvokeResponse Error(ToolInvokeRequest request, string message) => + new() + { + ToolCallId = request.ToolCallId, + ToolName = request.ToolName, + Content = message, + IsError = true + }; + + private static Dictionary ParseArguments(string? json) + { + if (string.IsNullOrWhiteSpace(json)) + return []; + return JsonSerializer.Deserialize>(json) ?? []; + } +} diff --git a/src/RockBot.Tools.FileSystem/FileSystemServiceCollectionExtensions.cs b/src/RockBot.Tools.FileSystem/FileSystemServiceCollectionExtensions.cs index 25d501bd..e56e021d 100644 --- a/src/RockBot.Tools.FileSystem/FileSystemServiceCollectionExtensions.cs +++ b/src/RockBot.Tools.FileSystem/FileSystemServiceCollectionExtensions.cs @@ -11,7 +11,7 @@ namespace RockBot.Tools.FileSystem; public static class FileSystemServiceCollectionExtensions { /// - /// Registers file tools for reading, writing, listing, and deleting files on the shared volume. + /// Registers file tools for reading, writing, editing, listing, and deleting files on the shared volume. /// public static AgentHostBuilder AddFileSystemTools( this AgentHostBuilder builder, diff --git a/src/RockBot.Tools.FileSystem/FileSystemToolRegistrar.cs b/src/RockBot.Tools.FileSystem/FileSystemToolRegistrar.cs index ccd34140..b676579d 100644 --- a/src/RockBot.Tools.FileSystem/FileSystemToolRegistrar.cs +++ b/src/RockBot.Tools.FileSystem/FileSystemToolRegistrar.cs @@ -38,6 +38,31 @@ internal sealed class FileSystemToolRegistrar( } """; + private const string EditSchema = """ + { + "type": "object", + "properties": { + "path": { + "type": "string", + "description": "Relative path within the shared volume (e.g. 'canon/NPCs.md')" + }, + "old_string": { + "type": "string", + "description": "Exact text to replace, copied verbatim from the file including whitespace and indentation. Must match exactly once unless replace_all is true." + }, + "new_string": { + "type": "string", + "description": "Text to replace it with. Use an empty string to delete the matched text." + }, + "replace_all": { + "type": "boolean", + "description": "Replace every occurrence instead of requiring a unique match. Defaults to false." + } + }, + "required": ["path", "old_string", "new_string"] + } + """; + private const string ListSchema = """ { "type": "object", @@ -87,6 +112,21 @@ public Task StartAsync(CancellationToken cancellationToken) }, new FileWriteToolExecutor(options)); logger.LogInformation("Registered file tool: file_write"); + registry.Register(new ToolRegistration + { + Name = "file_edit", + Description = """ + Replace an exact piece of text in an existing file on the shared volume, + leaving the rest of the file untouched. Prefer this over file_write when + changing part of a file — file_write replaces the entire file, so anything + not reproduced in full is lost. old_string must match exactly once unless + replace_all is set. + """, + ParametersSchema = EditSchema, + Source = "filesystem" + }, new FileEditToolExecutor(options)); + logger.LogInformation("Registered file tool: file_edit"); + registry.Register(new ToolRegistration { Name = "file_read", diff --git a/src/RockBot.Tools.FileSystem/FileSystemToolSkillProvider.cs b/src/RockBot.Tools.FileSystem/FileSystemToolSkillProvider.cs index 169d21d9..1f0148d5 100644 --- a/src/RockBot.Tools.FileSystem/FileSystemToolSkillProvider.cs +++ b/src/RockBot.Tools.FileSystem/FileSystemToolSkillProvider.cs @@ -8,13 +8,13 @@ namespace RockBot.Tools.FileSystem; internal sealed class FileSystemToolSkillProvider : IToolSkillProvider { public string Name => "files"; - public string Summary => "Shared-volume file tools (file_write, file_read, file_list, file_delete, file_get_path)."; + public string Summary => "Shared-volume file tools (file_write, file_edit, file_read, file_list, file_delete, file_get_path)."; public string GetDocument() => """ # Shared Volume File Tools Guide - Five tools provide direct access to files on the shared volume — a persistent + Six tools provide direct access to files on the shared volume — a persistent filesystem shared across the agent, script pods, and other RockBot services. @@ -27,11 +27,52 @@ Five tools provide direct access to files on the shared volume — a persistent - List or clean up files on the shared volume + ## Changing an Existing File: Edit, Don't Rewrite + + `file_write` replaces the **entire** file. Any content you do not reproduce in + full is gone — and on a long document you will not reliably reproduce it in full. + + So when a file already exists and you are changing part of it, use `file_edit`. + Reserve `file_write` for creating new files or deliberately replacing a whole + short one. + + This matters most for durable content — reference documents, notes, and records + that accumulate over time. Losing a paragraph from those is often invisible until + long after the edit. + + ## Tool Reference + ### file_edit + Replace an exact piece of text in an existing file, leaving everything else + byte-for-byte untouched. + + ``` + file_edit( + path: "canon/NPCs.md", + old_string: "**Georgie** — dock foreman, neutral", + new_string: "**Georgie** — dock foreman, owes the crew a favour" + ) + ``` + + Rules: + - `old_string` must match the file **exactly**, including whitespace and + indentation. Read the file first and copy the text verbatim rather than + reconstructing it from memory. + - `old_string` must match **exactly once**. If it appears more than once the edit + is refused — include more surrounding text to make the match unique, or pass + `replace_all: true` to change every occurrence. + - Use an empty `new_string` to delete the matched text. + - The file must already exist; use `file_write` to create it. + + A refused edit is information, not an obstacle. "Not found" means your `old_string` + does not match the file — re-read it rather than retrying the same text. "Occurs N + times" means you must disambiguate; do not switch to `file_write` to work around it. + ### file_write Write UTF-8 text to a file on the shared volume. Parent directories are created - automatically. + automatically. Replaces the whole file — see the section above before using it on + a file that already exists. ``` file_write(path: "drafts/report.md", content: "# Weekly Report\n...") @@ -79,6 +120,26 @@ tool requires a local path rather than content (e.g. uploading to OneDrive). - `scripts/` — output from script executions + ## Everything Expires by Default + + **Every file on the shared volume is deleted 30 days after it was last modified, + whatever directory it is in.** A path you invent yourself is not durable storage: + `canon/notes.md` will be swept 30 days after your last edit to it, exactly when it + has settled into being worth keeping. + + The sweep keys on modification time, so the files most at risk are the ones you + read often and edit rarely — which is what long-lived reference content looks like. + + Only paths an operator has added to the deployment's `shared.protectedPaths` + setting are exempt. You cannot set that yourself. So: + + - For content that must outlive 30 days without an edit, ask the user to have the + prefix protected, and say plainly that it will be deleted otherwise. + - Do not assume a directory is protected because it sounds permanent, or because + content you wrote there is still present today. + - Prefer memory tools over files for things you need to remember indefinitely. + + ## Working with Scripts Scripts run in ephemeral containers with the shared volume mounted at the path diff --git a/src/RockBot.Tools.FileSystem/FileText.cs b/src/RockBot.Tools.FileSystem/FileText.cs new file mode 100644 index 00000000..0c889d55 --- /dev/null +++ b/src/RockBot.Tools.FileSystem/FileText.cs @@ -0,0 +1,214 @@ +using System.Text; + +namespace RockBot.Tools.FileSystem; + +/// +/// Outcome of a call. +/// +/// Decoded text, or null when the file could not be decoded. +/// +/// The encoding the file was decoded with — pass it back to +/// so the file keeps its original form. +/// null on failure. +/// +/// +/// The file's raw bytes as read. Pass them to +/// to detect a concurrent writer. +/// null on failure. +/// +/// Human-readable failure description; null on success. +internal readonly record struct FileTextReadResult( + string? Content, + Encoding? Encoding, + byte[]? Bytes, + string? Error) +{ + /// Whether the file was decoded. + public bool IsSuccess => Content is not null; +} + +/// +/// Encoding-preserving, crash-safe text IO for in-place file edits. +/// +/// +/// +/// paired with +/// is not a +/// round trip: the read sniffs a byte-order mark while the write always emits UTF-8 +/// without one. Editing a single word in a UTF-16 document would silently re-encode +/// the whole file. An edit must change only what the caller asked to change, so the +/// encoding detected on read is carried back into the write. +/// +/// +/// A file with no BOM that is not valid UTF-8 is refused rather than decoded. The +/// permissive decoder maps every undecodable byte to U+FFFD, and persisting that +/// would corrupt an entire document to correct one line of it. +/// +/// +/// Writes land through a sibling temporary file and a rename. The obvious in-place +/// overwrite truncates the original before the replacement content is durable, so a +/// cancelled write — a subagent budget expiring, a pod eviction — would leave the +/// document empty. That is a worse failure than the whole-payload rewrite this tool +/// exists to avoid. +/// +/// +internal static class FileText +{ + /// + /// Reads as text, reporting the encoding it was decoded with. + /// + internal static async Task ReadAsync(string path, CancellationToken ct) + { + var bytes = await File.ReadAllBytesAsync(path, ct); + var encoding = DetectEncoding(bytes, out var preambleLength); + + try + { + var content = encoding.GetString(bytes, preambleLength, bytes.Length - preambleLength); + return new FileTextReadResult(content, encoding, bytes, null); + } + catch (DecoderFallbackException) + { + return new FileTextReadResult( + null, + null, + null, + "the file is not valid UTF-8 and carries no byte-order mark, so its text " + + "cannot be recovered without guessing an encoding. Editing it would corrupt " + + "every non-ASCII byte in the file, not just the edited region."); + } + } + + /// + /// Writes only if the file still holds + /// , returning false when it does not. + /// + /// + /// A read-modify-write cycle loses data when someone else writes in the middle of + /// it: both writers start from the same content and the last one to finish erases + /// the other's change. In-process callers are serialized by a lock, but a script + /// pod or MCP server sharing the volume is not, so the content is re-checked + /// immediately before the rename. This narrows the window rather than closing it — + /// a shared PVC offers no cross-process locking primitive — but it turns a silent + /// loss into a reported failure the caller can retry. + /// + internal static async Task WriteAtomicIfUnchangedAsync( + string path, + byte[] expectedBytes, + string content, + Encoding encoding, + CancellationToken ct) + { + var current = await File.ReadAllBytesAsync(path, ct); + if (!current.AsSpan().SequenceEqual(expectedBytes)) + return false; + + await WriteAtomicAsync(path, content, encoding, ct); + return true; + } + + /// + /// Writes to in + /// , replacing the file atomically. + /// + /// + /// The temporary file inherits the original's Unix mode before the rename, so an + /// edit does not change the permissions of the file it edits. + /// + internal static async Task WriteAtomicAsync( + string path, + string content, + Encoding encoding, + CancellationToken ct) + { + var directory = Path.GetDirectoryName(path)!; + var tempPath = Path.Combine(directory, $".{Path.GetFileName(path)}.{Guid.NewGuid():N}.tmp"); + + try + { + var preamble = encoding.GetPreamble(); + var body = encoding.GetBytes(content); + var bytes = new byte[preamble.Length + body.Length]; + preamble.CopyTo(bytes, 0); + body.CopyTo(bytes, preamble.Length); + + await File.WriteAllBytesAsync(tempPath, bytes, ct); + CopyUnixFileMode(path, tempPath); + File.Move(tempPath, path, overwrite: true); + } + catch + { + TryDelete(tempPath); + throw; + } + } + + /// + /// Identifies the encoding from a byte-order mark, defaulting to strict UTF-8. + /// + /// + /// Every returned encoding throws on invalid bytes rather than substituting U+FFFD, + /// and reproduces the file's original BOM (or absence of one) from + /// . + /// + private static Encoding DetectEncoding(byte[] bytes, out int preambleLength) + { + // UTF-32LE before UTF-16LE: both open with FF FE. + if (bytes.Length >= 4 && bytes[0] == 0xFF && bytes[1] == 0xFE && bytes[2] == 0x00 && bytes[3] == 0x00) + { + preambleLength = 4; + return new UTF32Encoding(bigEndian: false, byteOrderMark: true, throwOnInvalidCharacters: true); + } + + if (bytes.Length >= 4 && bytes[0] == 0x00 && bytes[1] == 0x00 && bytes[2] == 0xFE && bytes[3] == 0xFF) + { + preambleLength = 4; + return new UTF32Encoding(bigEndian: true, byteOrderMark: true, throwOnInvalidCharacters: true); + } + + if (bytes.Length >= 3 && bytes[0] == 0xEF && bytes[1] == 0xBB && bytes[2] == 0xBF) + { + preambleLength = 3; + return new UTF8Encoding(encoderShouldEmitUTF8Identifier: true, throwOnInvalidBytes: true); + } + + if (bytes.Length >= 2 && bytes[0] == 0xFF && bytes[1] == 0xFE) + { + preambleLength = 2; + return new UnicodeEncoding(bigEndian: false, byteOrderMark: true, throwOnInvalidBytes: true); + } + + if (bytes.Length >= 2 && bytes[0] == 0xFE && bytes[1] == 0xFF) + { + preambleLength = 2; + return new UnicodeEncoding(bigEndian: true, byteOrderMark: true, throwOnInvalidBytes: true); + } + + preambleLength = 0; + return new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: true); + } + + private static void CopyUnixFileMode(string source, string destination) + { + try + { + File.SetUnixFileMode(destination, File.GetUnixFileMode(source)); + } + catch + { + // Non-Unix platforms and filesystems without mode support — best-effort only. + } + } + + private static void TryDelete(string path) + { + try + { + File.Delete(path); + } + catch + { + // The temp file is already the failure path; nothing useful to add. + } + } +} diff --git a/tests/RockBot.Tools.FileSystem.Tests/FileEditToolExecutorTests.cs b/tests/RockBot.Tools.FileSystem.Tests/FileEditToolExecutorTests.cs new file mode 100644 index 00000000..554416cf --- /dev/null +++ b/tests/RockBot.Tools.FileSystem.Tests/FileEditToolExecutorTests.cs @@ -0,0 +1,341 @@ +using System.Text.Json; + +namespace RockBot.Tools.FileSystem.Tests; + +[TestClass] +public class FileEditToolExecutorTests +{ + private string _root = null!; + private FileSystemOptions _options = null!; + + [TestInitialize] + public void Setup() + { + _root = Path.Combine(Path.GetTempPath(), "rockbot-file-edit-tests", Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(_root); + _options = new FileSystemOptions { BasePath = _root }; + } + + [TestCleanup] + public void Cleanup() + { + if (Directory.Exists(_root)) + Directory.Delete(_root, recursive: true); + } + + private string WriteFile(string relativePath, string content) + { + var full = Path.Combine(_root, relativePath); + Directory.CreateDirectory(Path.GetDirectoryName(full)!); + File.WriteAllText(full, content); + return full; + } + + private static ToolInvokeRequest Request(object args) => new() + { + ToolCallId = "call_1", + ToolName = "file_edit", + Arguments = JsonSerializer.Serialize(args) + }; + + private Task ExecuteAsync(object args) => + new FileEditToolExecutor(_options).ExecuteAsync(Request(args), CancellationToken.None); + + [TestMethod] + public async Task ExecuteAsync_AppliesEditAndPersistsIt() + { + var full = WriteFile("canon/NPCs.md", "# NPCs\n\n**Georgie** — dock foreman, neutral\n"); + + var response = await ExecuteAsync(new + { + path = "canon/NPCs.md", + old_string = "dock foreman, neutral", + new_string = "dock foreman, owes the crew a favour" + }); + + Assert.IsFalse(response.IsError, response.Content); + Assert.AreEqual( + "# NPCs\n\n**Georgie** — dock foreman, owes the crew a favour\n", + File.ReadAllText(full)); + } + + [TestMethod] + public async Task ExecuteAsync_ReportsReplacementCountAndSizeDelta() + { + WriteFile("notes.md", "alpha beta gamma"); + + var response = await ExecuteAsync(new + { + path = "notes.md", + old_string = "beta", + new_string = "b" + }); + + Assert.IsFalse(response.IsError); + StringAssert.Contains(response.Content!, "Replaced 1 occurrence"); + StringAssert.Contains(response.Content!, "16 → 13"); + } + + [TestMethod] + public async Task ExecuteAsync_LeavesFileUntouched_WhenMatchIsAmbiguous() + { + const string original = "status: active\nstatus: active\n"; + var full = WriteFile("roster.md", original); + + var response = await ExecuteAsync(new + { + path = "roster.md", + old_string = "status: active", + new_string = "status: retired" + }); + + Assert.IsTrue(response.IsError); + StringAssert.Contains(response.Content!, "2 times"); + Assert.AreEqual(original, File.ReadAllText(full), "an ambiguous edit must not modify the file"); + } + + [TestMethod] + public async Task ExecuteAsync_LeavesFileUntouched_WhenOldStringNotFound() + { + const string original = "# Title\n\nbody\n"; + var full = WriteFile("doc.md", original); + + var response = await ExecuteAsync(new + { + path = "doc.md", + old_string = "missing text", + new_string = "replacement" + }); + + Assert.IsTrue(response.IsError); + StringAssert.Contains(response.Content!, "not found"); + Assert.AreEqual(original, File.ReadAllText(full)); + } + + [TestMethod] + public async Task ExecuteAsync_ReplacesAll_WhenReplaceAllIsTrue() + { + var full = WriteFile("roster.md", "status: active\nstatus: active\n"); + + var response = await ExecuteAsync(new + { + path = "roster.md", + old_string = "status: active", + new_string = "status: retired", + replace_all = true + }); + + Assert.IsFalse(response.IsError, response.Content); + StringAssert.Contains(response.Content!, "Replaced 2 occurrences"); + Assert.AreEqual("status: retired\nstatus: retired\n", File.ReadAllText(full)); + } + + [TestMethod] + public async Task ExecuteAsync_ReturnsError_WhenFileDoesNotExist() + { + var response = await ExecuteAsync(new + { + path = "nope.md", + old_string = "a", + new_string = "b" + }); + + Assert.IsTrue(response.IsError); + StringAssert.Contains(response.Content!, "File not found"); + StringAssert.Contains(response.Content!, "file_write"); + } + + [TestMethod] + public async Task ExecuteAsync_RejectsPathTraversal() + { + var outside = Path.Combine(Path.GetTempPath(), $"rockbot-outside-{Guid.NewGuid():N}.md"); + File.WriteAllText(outside, "secret"); + + try + { + var response = await ExecuteAsync(new + { + path = $"../{Path.GetFileName(outside)}", + old_string = "secret", + new_string = "leaked" + }); + + Assert.IsTrue(response.IsError); + StringAssert.Contains(response.Content!, "Invalid path"); + Assert.AreEqual("secret", File.ReadAllText(outside), "traversal must not modify files outside the volume"); + } + finally + { + File.Delete(outside); + } + } + + [TestMethod] + public async Task ExecuteAsync_ReturnsError_WhenRequiredArgumentMissing() + { + WriteFile("doc.md", "body"); + + var missingOld = await ExecuteAsync(new { path = "doc.md", new_string = "x" }); + Assert.IsTrue(missingOld.IsError); + StringAssert.Contains(missingOld.Content!, "old_string"); + + var missingNew = await ExecuteAsync(new { path = "doc.md", old_string = "body" }); + Assert.IsTrue(missingNew.IsError); + StringAssert.Contains(missingNew.Content!, "new_string"); + + var missingPath = await ExecuteAsync(new { old_string = "body", new_string = "x" }); + Assert.IsTrue(missingPath.IsError); + StringAssert.Contains(missingPath.Content!, "path"); + } + + [TestMethod] + public async Task ExecuteAsync_DeletesMatchedText_WhenNewStringIsEmpty() + { + var full = WriteFile("doc.md", "keep this\nDRAFT — remove me\nkeep that\n"); + + var response = await ExecuteAsync(new + { + path = "doc.md", + old_string = "DRAFT — remove me\n", + new_string = "" + }); + + Assert.IsFalse(response.IsError, response.Content); + Assert.AreEqual("keep this\nkeep that\n", File.ReadAllText(full)); + } + + [TestMethod] + public async Task ExecuteAsync_RejectsNullNewString_RatherThanDeleting() + { + const string original = "keep this text\n"; + var full = WriteFile("doc.md", original); + + var response = await ExecuteAsync(new + { + path = "doc.md", + old_string = "keep this text", + new_string = (string?)null + }); + + Assert.IsTrue(response.IsError); + StringAssert.Contains(response.Content!, "new_string must be a string"); + Assert.AreEqual(original, File.ReadAllText(full), + "a null new_string must not be treated as an intentional deletion"); + } + + [TestMethod] + public async Task ExecuteAsync_RejectsNonStringArguments() + { + WriteFile("doc.md", "body"); + + var nullOld = await ExecuteAsync(new { path = "doc.md", old_string = (string?)null, new_string = "x" }); + Assert.IsTrue(nullOld.IsError); + StringAssert.Contains(nullOld.Content!, "old_string must be a string"); + + var numericPath = await ExecuteAsync(new { path = 7, old_string = "body", new_string = "x" }); + Assert.IsTrue(numericPath.IsError); + StringAssert.Contains(numericPath.Content!, "path must be a string"); + } + + [TestMethod] + public async Task ExecuteAsync_AcceptsStringSpellingOfReplaceAll() + { + // The text-based tool-calling path has no schema to coerce "true" to true. + var full = WriteFile("roster.md", "status: active\nstatus: active\n"); + + var response = await ExecuteAsync(new + { + path = "roster.md", + old_string = "status: active", + new_string = "status: retired", + replace_all = "true" + }); + + Assert.IsFalse(response.IsError, response.Content); + Assert.AreEqual("status: retired\nstatus: retired\n", File.ReadAllText(full)); + } + + [TestMethod] + public async Task ExecuteAsync_RejectsUnparseableReplaceAll() + { + const string original = "status: active\nstatus: active\n"; + var full = WriteFile("roster.md", original); + + var response = await ExecuteAsync(new + { + path = "roster.md", + old_string = "status: active", + new_string = "status: retired", + replace_all = "yes" + }); + + Assert.IsTrue(response.IsError); + StringAssert.Contains(response.Content!, "replace_all must be true or false"); + Assert.AreEqual(original, File.ReadAllText(full)); + } + + [TestMethod] + public async Task ExecuteAsync_TreatsAbsentAndFalseReplaceAllAlike() + { + WriteFile("roster.md", "status: active\nstatus: active\n"); + + var explicitFalse = await ExecuteAsync(new + { + path = "roster.md", + old_string = "status: active", + new_string = "status: retired", + replace_all = false + }); + + Assert.IsTrue(explicitFalse.IsError); + StringAssert.Contains(explicitFalse.Content!, "2 times"); + } + + [TestMethod] + public async Task ExecuteAsync_AppliesBothEdits_WhenSameFileIsEditedConcurrently() + { + // Two subagents editing one document must not have the second read stale + // content and overwrite the first — both edits belong in the result. + var full = WriteFile("canon/NPCs.md", "**Georgie** — neutral\n**Wren** — neutral\n"); + + var georgie = ExecuteAsync(new + { + path = "canon/NPCs.md", + old_string = "**Georgie** — neutral", + new_string = "**Georgie** — allied" + }); + var wren = ExecuteAsync(new + { + path = "canon/NPCs.md", + old_string = "**Wren** — neutral", + new_string = "**Wren** — hostile" + }); + + var responses = await Task.WhenAll(georgie, wren); + + foreach (var response in responses) + Assert.IsFalse(response.IsError, response.Content); + + Assert.AreEqual("**Georgie** — allied\n**Wren** — hostile\n", File.ReadAllText(full)); + } + + [TestMethod] + public async Task ExecuteAsync_PreservesUnrelatedContentOfLargeFile() + { + var lines = Enumerable.Range(0, 500).Select(i => $"## Section {i}\nBody text for section {i}.\n"); + var original = string.Concat(lines); + var full = WriteFile("canon/big.md", original); + + var response = await ExecuteAsync(new + { + path = "canon/big.md", + old_string = "Body text for section 250.", + new_string = "Rewritten body for section 250." + }); + + Assert.IsFalse(response.IsError, response.Content); + var edited = File.ReadAllText(full); + Assert.AreEqual(original.Replace("Body text for section 250.", "Rewritten body for section 250."), edited); + Assert.AreEqual(original.Length + 5, edited.Length); + } +} diff --git a/tests/RockBot.Tools.FileSystem.Tests/FileTextTests.cs b/tests/RockBot.Tools.FileSystem.Tests/FileTextTests.cs new file mode 100644 index 00000000..840d4083 --- /dev/null +++ b/tests/RockBot.Tools.FileSystem.Tests/FileTextTests.cs @@ -0,0 +1,214 @@ +using System.Text; + +namespace RockBot.Tools.FileSystem.Tests; + +[TestClass] +public class FileTextTests +{ + private string _root = null!; + + [TestInitialize] + public void Setup() + { + _root = Path.Combine(Path.GetTempPath(), "rockbot-file-text-tests", Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(_root); + } + + [TestCleanup] + public void Cleanup() + { + if (Directory.Exists(_root)) + Directory.Delete(_root, recursive: true); + } + + private string Path_(string name) => Path.Combine(_root, name); + + /// + /// Reads the file, applies a replacement, writes it back — the exact cycle + /// performs. + /// + private static async Task RoundTripAsync(string path, string oldText, string newText) + { + var read = await FileText.ReadAsync(path, CancellationToken.None); + Assert.IsTrue(read.IsSuccess, read.Error); + await FileText.WriteAtomicAsync( + path, + read.Content!.Replace(oldText, newText, StringComparison.Ordinal), + read.Encoding!, + CancellationToken.None); + return await File.ReadAllBytesAsync(path); + } + + [TestMethod] + public async Task RoundTrip_PreservesUtf8WithoutBom() + { + var path = Path_("plain.md"); + await File.WriteAllBytesAsync(path, new UTF8Encoding(false).GetBytes("alpha — beta\n")); + + var bytes = await RoundTripAsync(path, "beta", "gamma"); + + CollectionAssert.AreEqual(new UTF8Encoding(false).GetBytes("alpha — gamma\n"), bytes); + Assert.AreNotEqual(0xEF, bytes[0], "a BOM must not be introduced"); + } + + [TestMethod] + public async Task RoundTrip_PreservesUtf8Bom() + { + var path = Path_("bom.md"); + await File.WriteAllBytesAsync(path, new UTF8Encoding(true).GetPreamble() + .Concat(new UTF8Encoding(false).GetBytes("alpha beta\n")).ToArray()); + + var bytes = await RoundTripAsync(path, "beta", "gamma"); + + CollectionAssert.AreEqual(new byte[] { 0xEF, 0xBB, 0xBF }, bytes.Take(3).ToArray()); + Assert.AreEqual("alpha gamma\n", new UTF8Encoding(false).GetString(bytes, 3, bytes.Length - 3)); + } + + [TestMethod] + public async Task RoundTrip_PreservesUtf16LittleEndian() + { + // The shape a Windows PowerShell 5.1 producer leaves on the shared volume. + var path = Path_("utf16.md"); + var encoding = new UnicodeEncoding(bigEndian: false, byteOrderMark: true); + await File.WriteAllBytesAsync(path, encoding.GetPreamble() + .Concat(encoding.GetBytes("alpha — beta\n")).ToArray()); + + var bytes = await RoundTripAsync(path, "beta", "gamma"); + + CollectionAssert.AreEqual(new byte[] { 0xFF, 0xFE }, bytes.Take(2).ToArray()); + Assert.AreEqual("alpha — gamma\n", encoding.GetString(bytes, 2, bytes.Length - 2)); + } + + [TestMethod] + public async Task RoundTrip_PreservesUtf16BigEndian() + { + var path = Path_("utf16be.md"); + var encoding = new UnicodeEncoding(bigEndian: true, byteOrderMark: true); + await File.WriteAllBytesAsync(path, encoding.GetPreamble() + .Concat(encoding.GetBytes("alpha beta\n")).ToArray()); + + var bytes = await RoundTripAsync(path, "beta", "gamma"); + + CollectionAssert.AreEqual(new byte[] { 0xFE, 0xFF }, bytes.Take(2).ToArray()); + Assert.AreEqual("alpha gamma\n", encoding.GetString(bytes, 2, bytes.Length - 2)); + } + + [TestMethod] + public async Task RoundTrip_PreservesUtf32LittleEndian() + { + // FF FE 00 00 must be read as UTF-32, not as UTF-16LE followed by a NUL. + var path = Path_("utf32.md"); + var encoding = new UTF32Encoding(bigEndian: false, byteOrderMark: true); + await File.WriteAllBytesAsync(path, encoding.GetPreamble() + .Concat(encoding.GetBytes("alpha beta\n")).ToArray()); + + var bytes = await RoundTripAsync(path, "beta", "gamma"); + + CollectionAssert.AreEqual(new byte[] { 0xFF, 0xFE, 0x00, 0x00 }, bytes.Take(4).ToArray()); + Assert.AreEqual("alpha gamma\n", encoding.GetString(bytes, 4, bytes.Length - 4)); + } + + [TestMethod] + public async Task ReadAsync_RefusesUndecodableBytes_RatherThanSubstituting() + { + // Latin-1 "café" — 0xE9 is not valid UTF-8 and there is no BOM to disambiguate. + var path = Path_("latin1.md"); + await File.WriteAllBytesAsync(path, [0x63, 0x61, 0x66, 0xE9, 0x0A]); + + var read = await FileText.ReadAsync(path, CancellationToken.None); + + Assert.IsFalse(read.IsSuccess); + StringAssert.Contains(read.Error!, "not valid UTF-8"); + CollectionAssert.AreEqual( + new byte[] { 0x63, 0x61, 0x66, 0xE9, 0x0A }, + await File.ReadAllBytesAsync(path), + "a refused read must leave the file alone"); + } + + [TestMethod] + public async Task WriteAtomicAsync_LeavesNoTempFileBehind() + { + var path = Path_("doc.md"); + await File.WriteAllTextAsync(path, "body\n"); + + await RoundTripAsync(path, "body", "text"); + + CollectionAssert.AreEqual( + new[] { "doc.md" }, + Directory.GetFiles(_root).Select(Path.GetFileName).ToArray()); + } + + [TestMethod] + public async Task WriteAtomicAsync_PreservesOriginal_WhenCancelled() + { + var path = Path_("doc.md"); + const string original = "durable content\n"; + await File.WriteAllTextAsync(path, original); + + using var cts = new CancellationTokenSource(); + await cts.CancelAsync(); + + await Assert.ThrowsExactlyAsync(() => + FileText.WriteAtomicAsync(path, "replacement", new UTF8Encoding(false), cts.Token)); + + Assert.AreEqual(original, await File.ReadAllTextAsync(path), + "a cancelled write must not truncate the file it was replacing"); + CollectionAssert.AreEqual( + new[] { "doc.md" }, + Directory.GetFiles(_root).Select(Path.GetFileName).ToArray()); + } + + [TestMethod] + public async Task WriteAtomicIfUnchangedAsync_RefusesWrite_WhenFileChangedSinceRead() + { + var path = Path_("doc.md"); + await File.WriteAllTextAsync(path, "original\n"); + + var read = await FileText.ReadAsync(path, CancellationToken.None); + Assert.IsTrue(read.IsSuccess); + + // Another writer lands between the read and the write. + await File.WriteAllTextAsync(path, "someone else's change\n"); + + var written = await FileText.WriteAtomicIfUnchangedAsync( + path, read.Bytes!, "my edit\n", read.Encoding!, CancellationToken.None); + + Assert.IsFalse(written); + Assert.AreEqual("someone else's change\n", await File.ReadAllTextAsync(path), + "the other writer's change must survive"); + } + + [TestMethod] + public async Task WriteAtomicIfUnchangedAsync_Writes_WhenFileIsUntouched() + { + var path = Path_("doc.md"); + await File.WriteAllTextAsync(path, "original\n"); + + var read = await FileText.ReadAsync(path, CancellationToken.None); + var written = await FileText.WriteAtomicIfUnchangedAsync( + path, read.Bytes!, "my edit\n", read.Encoding!, CancellationToken.None); + + Assert.IsTrue(written); + Assert.AreEqual("my edit\n", await File.ReadAllTextAsync(path)); + } + + [TestMethod] + public async Task WriteAtomicAsync_PreservesUnixFileMode() + { + if (OperatingSystem.IsWindows()) + Assert.Inconclusive("Unix file modes are not meaningful on Windows."); + + var path = Path_("doc.md"); + await File.WriteAllTextAsync(path, "body\n"); + const UnixFileMode mode = + UnixFileMode.UserRead | UnixFileMode.UserWrite | + UnixFileMode.GroupRead | UnixFileMode.GroupWrite | + UnixFileMode.OtherRead | UnixFileMode.OtherWrite; + File.SetUnixFileMode(path, mode); + + await RoundTripAsync(path, "body", "text"); + + Assert.AreEqual(mode, File.GetUnixFileMode(path), + "editing a file must not change its permissions"); + } +} diff --git a/tests/RockBot.Tools.FileSystem.Tests/RockBot.Tools.FileSystem.Tests.csproj b/tests/RockBot.Tools.FileSystem.Tests/RockBot.Tools.FileSystem.Tests.csproj new file mode 100644 index 00000000..6adb0823 --- /dev/null +++ b/tests/RockBot.Tools.FileSystem.Tests/RockBot.Tools.FileSystem.Tests.csproj @@ -0,0 +1,23 @@ + + + + net10.0 + enable + enable + false + true + + + + + + + + + + + + + + + diff --git a/tests/RockBot.Tools.FileSystem.Tests/TextEditTests.cs b/tests/RockBot.Tools.FileSystem.Tests/TextEditTests.cs new file mode 100644 index 00000000..8387ab00 --- /dev/null +++ b/tests/RockBot.Tools.FileSystem.Tests/TextEditTests.cs @@ -0,0 +1,250 @@ +using RockBot.Host; + +namespace RockBot.Tools.FileSystem.Tests; + +[TestClass] +public class TextEditTests +{ + [TestMethod] + public void Apply_ReplacesSingleOccurrence() + { + var result = TextEdit.Apply("the quick brown fox", "brown", "red"); + + Assert.IsTrue(result.IsSuccess); + Assert.AreEqual("the quick red fox", result.Content); + Assert.AreEqual(1, result.ReplacementCount); + Assert.IsNull(result.Error); + } + + [TestMethod] + public void Apply_LeavesRestOfContentByteForByte() + { + var original = "# Title\n\nPara one.\n\n- item a\n- item b\n\n## Sub\n\ttabbed spaced\n"; + + var result = TextEdit.Apply(original, "item a", "item A"); + + Assert.IsTrue(result.IsSuccess); + Assert.AreEqual(original.Replace("item a", "item A"), result.Content); + } + + [TestMethod] + public void Apply_ReturnsNotFound_WhenOldTextAbsent() + { + var result = TextEdit.Apply("hello world", "goodbye", "farewell"); + + Assert.AreEqual(TextEditStatus.NotFound, result.Status); + Assert.IsNull(result.Content); + Assert.AreEqual(0, result.ReplacementCount); + StringAssert.Contains(result.Error!, "not found"); + } + + [TestMethod] + public void Apply_ReturnsAmbiguous_WhenMultipleMatchesAndNotReplaceAll() + { + var result = TextEdit.Apply("cat dog cat", "cat", "bird"); + + Assert.AreEqual(TextEditStatus.Ambiguous, result.Status); + Assert.IsNull(result.Content); + StringAssert.Contains(result.Error!, "2 times"); + } + + [TestMethod] + public void Apply_ReplacesEveryOccurrence_WhenReplaceAll() + { + var result = TextEdit.Apply("cat dog cat", "cat", "bird", replaceAll: true); + + Assert.IsTrue(result.IsSuccess); + Assert.AreEqual("bird dog bird", result.Content); + Assert.AreEqual(2, result.ReplacementCount); + } + + [TestMethod] + public void Apply_ReturnsEmptyOldText_WhenOldTextIsEmpty() + { + var result = TextEdit.Apply("content", "", "new"); + + Assert.AreEqual(TextEditStatus.EmptyOldText, result.Status); + Assert.IsNull(result.Content); + } + + [TestMethod] + public void Apply_ReturnsNoChange_WhenOldAndNewAreIdentical() + { + var result = TextEdit.Apply("content here", "here", "here"); + + Assert.AreEqual(TextEditStatus.NoChange, result.Status); + Assert.IsNull(result.Content); + } + + [TestMethod] + public void Apply_DeletesText_WhenNewTextIsEmpty() + { + var result = TextEdit.Apply("keep this, drop that", ", drop that", ""); + + Assert.IsTrue(result.IsSuccess); + Assert.AreEqual("keep this", result.Content); + } + + [TestMethod] + public void Apply_IsCaseSensitive() + { + var result = TextEdit.Apply("Hello World", "hello", "goodbye"); + + Assert.AreEqual(TextEditStatus.NotFound, result.Status); + } + + [TestMethod] + public void Apply_CountsNonOverlappingOccurrences() + { + // "aaaa" contains "aa" twice non-overlapping, not three times overlapping. + var result = TextEdit.Apply("aaaa", "aa", "b", replaceAll: true); + + Assert.IsTrue(result.IsSuccess); + Assert.AreEqual("bb", result.Content); + Assert.AreEqual(2, result.ReplacementCount); + } + + [TestMethod] + public void Apply_MatchesAcrossLines() + { + var original = "line one\nline two\nline three\n"; + + var result = TextEdit.Apply(original, "line one\nline two", "line 1\nline 2"); + + Assert.IsTrue(result.IsSuccess); + Assert.AreEqual("line 1\nline 2\nline three\n", result.Content); + } + + [TestMethod] + public void Apply_DisambiguatesWithSurroundingContext() + { + var original = "## Alice\nstatus: active\n\n## Bob\nstatus: active\n"; + + // "status: active" alone is ambiguous... + var ambiguous = TextEdit.Apply(original, "status: active", "status: retired"); + Assert.AreEqual(TextEditStatus.Ambiguous, ambiguous.Status); + + // ...but including the heading makes it unique. + var result = TextEdit.Apply(original, "## Bob\nstatus: active", "## Bob\nstatus: retired"); + Assert.IsTrue(result.IsSuccess); + Assert.AreEqual("## Alice\nstatus: active\n\n## Bob\nstatus: retired\n", result.Content); + } + + [TestMethod] + public void Apply_MatchesLfOldTextAgainstCrLfContent() + { + var original = "line one\r\nline two\r\nline three\r\n"; + + var result = TextEdit.Apply(original, "line one\nline two", "line 1\nline 2"); + + Assert.IsTrue(result.IsSuccess); + Assert.AreEqual("line 1\r\nline 2\r\nline three\r\n", result.Content); + } + + [TestMethod] + public void Apply_PrefersExactMatch_OverCrLfFallback() + { + // Content has a bare-LF region and a CRLF region. An LF oldText must hit the + // literal LF match rather than being converted and hitting the CRLF one. + var original = "alpha\nbeta\r\nalpha\r\nbeta\r\n"; + + var result = TextEdit.Apply(original, "alpha\nbeta", "X"); + + Assert.IsTrue(result.IsSuccess); + Assert.AreEqual("X\r\nalpha\r\nbeta\r\n", result.Content); + Assert.AreEqual(1, result.ReplacementCount); + } + + [TestMethod] + public void Apply_MatchesCrLfOldTextAgainstLfContent() + { + // The reverse of the LF-against-CRLF case: a caller reading through a + // CRLF-normalizing source editing a Unix-authored document. + var original = "line one\nline two\nline three\n"; + + var result = TextEdit.Apply(original, "line one\r\nline two", "line 1\r\nline 2"); + + Assert.IsTrue(result.IsSuccess); + Assert.AreEqual("line 1\nline 2\nline three\n", result.Content); + } + + [TestMethod] + public void Apply_ConvertsNewTextToCrLf_OnExactMatchPath() + { + // oldText matches without conversion, so only newText carries foreign endings. + // Inserting it verbatim would leave the document with mixed line endings. + var original = "alpha\r\nbeta\r\n"; + + var result = TextEdit.Apply(original, "beta", "beta\nand gamma"); + + Assert.IsTrue(result.IsSuccess); + Assert.AreEqual("alpha\r\nbeta\r\nand gamma\r\n", result.Content); + Assert.IsFalse(HasBareLf(result.Content!), "a CRLF document must stay CRLF"); + } + + [TestMethod] + public void Apply_ConvertsNewTextToLf_OnExactMatchPath() + { + var original = "alpha\nbeta\n"; + + var result = TextEdit.Apply(original, "beta", "beta\r\nand gamma"); + + Assert.IsTrue(result.IsSuccess); + Assert.AreEqual("alpha\nbeta\nand gamma\n", result.Content); + } + + [TestMethod] + public void Apply_LeavesNewTextAlone_WhenContentHasMixedEndings() + { + // Mixed content has no single style to conform to, so imposing one would + // rewrite line endings the caller never asked to touch. + var original = "alpha\r\nbeta\ngamma\r\n"; + + var result = TextEdit.Apply(original, "gamma", "gamma\ndelta"); + + Assert.IsTrue(result.IsSuccess); + Assert.AreEqual("alpha\r\nbeta\ngamma\ndelta\r\n", result.Content); + } + + [TestMethod] + public void Apply_ReturnsNoChange_WhenTextsDifferOnlyInLineEndings() + { + var result = TextEdit.Apply("alpha\r\nbeta\r\n", "alpha\r\nbeta", "alpha\nbeta"); + + Assert.AreEqual(TextEditStatus.NoChange, result.Status); + Assert.IsNull(result.Content); + StringAssert.Contains(result.Error!, "line endings"); + } + + private static bool HasBareLf(string value) + { + for (var i = 0; i < value.Length; i++) + { + if (value[i] == '\n' && (i == 0 || value[i - 1] != '\r')) + return true; + } + + return false; + } + + [TestMethod] + public void Apply_DoesNotAppendOrStripTrailingNewline() + { + var withNewline = TextEdit.Apply("body\n", "body", "text"); + Assert.AreEqual("text\n", withNewline.Content); + + var withoutNewline = TextEdit.Apply("body", "body", "text"); + Assert.AreEqual("text", withoutNewline.Content); + } + + [TestMethod] + public void Apply_PreservesUnicodeContent() + { + var original = "Turkana — long-lived\nNarvik — predatory\n"; + + var result = TextEdit.Apply(original, "Narvik — predatory", "Narvik — fast, predatory"); + + Assert.IsTrue(result.IsSuccess); + Assert.AreEqual("Turkana — long-lived\nNarvik — fast, predatory\n", result.Content); + } +}