Add file_edit tool and protect durable paths from the shared-volume sweep - #504
Merged
Conversation
…weep Every RockBot write surface replaces its entire payload, so changing one line of a document means re-emitting the whole document — and anything the model fails to reproduce is silently lost. That risk scales with document length and with how long the content has been accumulating. Adds TextEdit, an exact-match replacement primitive in RockBot.Host, and wires it to a new file_edit tool on the shared volume. Ambiguity is an error rather than a guess: when old_string matches more than once the edit is refused, because a tool that silently edits the first of several matches gives the caller no way to know which one it hit. When the file is CRLF and old_string arrives with bare LFs, the match is retried with converted newlines so callers need not discover line-ending style by trial and error. TextEdit is deliberately separate from the file plumbing so the memory, working-memory, and skill surfaces named in #502 can reuse it unchanged. Also exempts configured prefixes from the shared-volume cleanup sweep. The catch-all keys on mtime, so it deletes files for not having changed recently — precisely the state of durable reference content the agent reads far more often than it edits. protectedPaths defaults to empty, leaving existing deployments unchanged. Uses `!` rather than `-not`, which busybox does not reliably provide, and the glob covers nested content so a `.git` directory under a protected prefix survives too. Partial fix for #502; the memory surfaces follow separately. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ed paths everywhere Follow-up to the file_edit review on #504. Ten findings, all addressed. Encoding round-trip. ReadAllTextAsync sniffs a BOM, WriteAllTextAsync always emits UTF-8 without one, so a one-word edit silently re-encoded a UTF-16 file and mapped every undecodable byte of a Latin-1 file to U+FFFD. New FileText detects the encoding from the BOM and writes it back with the same preamble; a no-BOM file that is not valid UTF-8 is refused rather than decoded, since corrupting a whole document to fix one line of it is the worse failure. Atomicity. The in-place overwrite truncated the original before the replacement was durable, so a cancelled write left the file empty — strictly worse than the whole-payload loss file_edit exists to prevent. Writes now land via a sibling temp file and a rename. Renaming succeeds on a writable directory even when the file itself is not writable, which would have widened the tool past its documented permission boundary, so the file is probed for write access first and the UID mismatch still reports Permission denied. Concurrency. Read-modify-write lost an edit when two subagents touched one document, both reporting success. Same-path edits now serialize on a lock, and the content is re-checked immediately before the rename so a cross-process writer is reported instead of silently overwritten. Argument handling. A JSON null new_string was coalesced to "" and deleted the matched text; non-string arguments are now rejected. replace_all accepted only a JSON boolean, so the string "true" a text-based tool call emits was ignored and the edit refused as ambiguous with nothing to explain why. Line endings. The CRLF retry was gated on the file containing CRLF, so a CRLF old_string against an LF file fell through to "not found" telling the caller to copy the text verbatim, which it had. Both directions are handled now. new_text is also conformed to the file's existing style on the exact-match path, not just the retry path, so an edit can no longer leave a single-style document mixed. Content that is already mixed is left alone. protectedPaths. Entries were interpolated raw: a trailing slash rendered '.../notes//*', which fnmatch cannot match, silently deleting the content the operator had listed. Verified against busybox — the old clauses deleted both a 'notes/' directory and a protected leaf file. Entries are now slash-normalized, cover a leaf file as well as a directory's contents, and apply to the per-directory sweeps too rather than the catch-all alone. Skill guide. It told the agent to use file_edit for durable content without mentioning that everything on the volume is swept 30 days after its last modification, so the agent would confidently create canon/notes.md and lose it. Verification: 2,619 tests pass (was 2,598) — 22 new across TextEdit, FileText, and the executor. Helm renders correctly with empty, trailing-slash, leading-slash, and leaf-file entries, and byte-identically to before when protectedPaths is empty. Sweep behaviour checked against a real busybox:stable container in both the protected and unprotected directions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # Directory.Build.props
# Conflicts: # Directory.Build.props
Merged
rockfordlhotka
added a commit
that referenced
this pull request
Aug 10, 2026
PR #504 changed the chart templates — the shared-cleanup CronJob command, the protectedPaths helper in _helpers.tpl, and the shared.protectedPaths default in values.yaml — but left Chart.yaml at 0.10.17. The chart version therefore no longer distinguished a chart that honours protectedPaths from one that does not. Bumps version and appVersion in lockstep, matching the convention in #503, #400, and #320. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Partial fix for #502 — the shared-volume file surface. The memory, working-memory, and skill surfaces follow in a second PR, deliberately split so this one did not collide with the recall-tool consolidation in #501.
Why
Every RockBot write surface replaces its entire payload. Changing one line of a document means re-emitting the whole document, and anything the model fails to reproduce is silently lost. The risk scales with document length and with how long the content has been accumulating — which is exactly backwards, because the longest and oldest documents are the ones worth protecting most.
What
TextEdit(src/RockBot.Host/TextEdit.cs) — an exact-match replacement primitive, kept separate from the file plumbing so the memory surfaces in #502 can reuse it unchanged. Returns a status enum rather than throwing, so each caller can phrase its own error.The load-bearing design decision: ambiguity is an error, not a guess. When
old_stringmatches more than once the edit is refused. A tool that silently edits the first of several matches gives the caller no way to know which one it hit, which is worse than one that declines.replace_allis the explicit opt-in.It also handles the CRLF/LF mismatch: when the file is CRLF and
old_stringarrives with bare LFs, the match is retried with converted newlines. Exact-matching against the wrong newline style is indistinguishable from "text not found", so without this the failure is confusing rather than informative.file_edittool — registered alongside the existing five, plus a skill-guide section directing the agent to prefer it overfile_writefor existing files, and explaining that a refused edit is information ("not found" means re-read the file, not retry the same string).shared.protectedPaths— exempts configured prefixes from the shared-volume cleanup sweep. The catch-all keys onmtime, so it deletes files for not having changed recently — the normal state of durable reference content the agent reads far more often than it edits. Defaults to[], so existing deployments render byte-identically.Two details worth noting:
!rather than-not, which busybox does not reliably provide; and the glob covers nested content, so a.gitdirectory under a protected prefix survives — without that, the sweep would delete loose objects older than 30 days and quietly corrupt the repo intended as the backup.Verification
RockBot.Tools.FileSystem.Testsproject (theInternalsVisibleTofor it already existed)busybox:stablecontainer: a 2020-mtime file under a protected prefix and its.gitobjects survive, whiledrafts/stale.mdand an unprotected ad-hoc file are still sweptfile_editconfirmed registered in agent logsOperational note
file_editdeliberately does not re-assertSetUnixFileMode, unlikefile_write— writing creates files, editing should not silently change permissions on an existing one.Consequence: a file placed on the PVC by a different UID (script pods run as 1000, the agent as 999) is readable but not editable by the agent. Observed on the live volume, the root carries
fsGroup2000 but has no setgid bit, so new files take their creator's primary GID at mode 644 rather than the shared group. Worth knowing before bulk-loading content that the agent is expected to edit.🤖 Generated with Claude Code