bugFix(tools): refuse edits to PDF and DOCX documents - #1125
Open
addyCooks wants to merge 1 commit into
Open
Conversation
Reading a .pdf or .docx returns a markdown transcript rather than the file's bytes, and the write path had no matching branch: string_replace, diff_edit and write_file applied the edit to that transcript and wrote it back over the document as UTF-8. A request to fix one word replaced a real document with a few hundred bytes of plain text, and the tool reported success. Neither undo system could recover it -- checkpoints skip binaries and file snapshots store text -- so the original bytes were gone the moment the write landed. The three write tools now refuse any path whose content the read path can only transcribe, naming the reason so the model stops instead of retrying. The check is on the extension, so it also covers a fresh write_file to a .pdf that was never read. CachedFile additionally carries a derived flag, so a caller holding cached content can tell whether it is the file or a transcript of it. Closes Nano-Collective#1058
addyCooks
requested review from
Avtrkrb,
akramcodez and
will-lamerton
as code owners
September 1, 2026 19:22
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.
Closes #1058
Description
getCachedFileContentis the single read path for the edit tools, and for.pdfand
.docxit returns a markdown transcript instead of the file's bytes. The writeside had no matching branch, and
CachedFilecarried nothing that let a caller tellderived content from real content so no caller could check.
Changes:
isDerivedContentPath()infile-cache.tsis now the single source of truth forwhich formats are transcribed. The conversion branch uses it too, so the read and
write sides cannot drift apart as formats are added. A duplicate
.pdf/.docxcheck in
read-file.tsxwas folded into it.CachedFilegained aderivedflag, closing the gap the issue named.validateEditableFormat()inpath-validators.tsis the shared guard, wired intothe validator and the executor of all three write tools. The executor copy is
not redundant:
ToolRegistry.fromToolExportsregisters the rawtoolalongsidethe
withValidation-wrapped handler, soexecuteis reachable without thevalidator.
fresh
write_fileto a.pdfthat was never read writing UTF-8 text to thosepaths is never correct.
getCachedFileContent, so a refused edit never pays for adocument conversion.
Scope note: #962 covers the same class of loss in checkpoints (UTF-8 round-tripping
destroying binaries on save). This PR is the edit-tool path only; the two fixes are
independent.
Type of Change
Changeset
pnpm changeset) describing this change for the changelogTesting
Automated Tests
.spec.ts/tsxfilespnpm test:allcompletes successfully)Manual Testing
Checklist