Skip to content

[Bug] lsp_format_document rewrites non-UTF-8 bytes as U+FFFD, and read_file labels such files UTF-8 (follow-up to #1854) #1867

Open

Description

@diyaavirmani

Description

Follow-up to #1854. While reviewing the fix for it (#1866), the automated reviewer found two more places that mishandle a file that isn't valid UTF-8, such as Latin-1, Windows-1252 or Shift-JIS. I reproduced both. #1866 doesn't cover either, because #1854 was about string_replace and diff_edit.

  1. lsp_format_document rewrites every non-UTF-8 byte as U+FFFD. It reads the file as UTF-8, applies the language server's edits, and writes the whole file back as UTF-8, the same way string_replace did. So formatting a Latin-1 file changes bytes the formatter never touched, and the tool reports "Formatted … (1 edit applied)".
  2. read_file with metadata_only reports Encoding: UTF-8 for a file that isn't UTF-8. It decides the encoding by trying readFile(absPath, 'utf-8'), which never fails: Node replaces invalid bytes with U+FFFD instead of throwing. So the label is always UTF-8 for a regular file, which tells the model a Latin-1 file is safe to treat as UTF-8.

This is narrower than #1854. The formatter only runs when the model calls lsp_format_document, and language servers only start in the interactive TUI: --plain run and ACP never call getLSPManager() with the server config. It also needs a language server with formatting support for that file type. Still, it's the same silent damage, and the user asked for formatting, not for a change of encoding.

Environment

  • OS: macOS (Darwin 25.6.0, x86_64)
  • Node version: 22.23.3
  • Nanocoder version: main at c72945d3, v1.31.0, and the fix(tools): refuse to edit a file that isn't valid UTF-8 (#1854) #1866 branch
  • Provider: a local OpenAI-compatible test server with scripted responses
  • Model: n/a (scripted)
  • Language server: a minimal stdio test server that advertises document formatting and returns one edit (old → new on line 2), configured through lspServers

Steps to Reproduce

Formatter (interactive TUI):

  1. In a scratch project, create a Latin-1 file printf 'caf\351\nold\n' > legacy.legacy. Line 1 is café in Latin-1, a single E9 byte.
  2. Configure a language server for the legacy extension in lspServers. Any server with documentFormattingProvider works. Mine returns one edit replacing old with new on line 2.
  3. Start nanocoder --mode auto-accept, trust the folder, and have the model call lsp_format_document on legacy.legacy.
  4. Compare the bytes: xxd -p legacy.legacy.

Encoding label:

  1. With the same Latin-1 file, have the model call read_file with {"path": "legacy.legacy", "metadata_only": true}. I used nanocoder --trust-directory --mode auto-accept --plain run.
  2. Check the Encoding: line.

Expected Behavior

Actual Behavior

Case v1.31.0 main c72945d3 #1866 branch
format a Latin-1 file: 63 61 66 e9 0a 6f 6c 64 0a 63 61 66 ef bf bd 0a 6e 65 77 0a, "Formatted legacy.legacy (1 edit applied)" same same
format the same text saved as UTF-8 (control): 63 61 66 c3 a9 0a 6f 6c 64 0a 63 61 66 c3 a9 0a 6e 65 77 0a same same
read_file metadata_only on the Latin-1 file Encoding: UTF-8 same same

In the formatter case, line 1 was never part of the edit, but its E9 byte became EF BF BD. The language server saw initialize, didOpen, formatting, then didChange with the damaged text.

Logs/Screenshots

What I think is going on:

  • formatFileWithLsp reads with readFile(absPath, 'utf-8') and writes with writeFile(absPath, formatted, 'utf-8') (source/tools/lsp-format-document.tsx:292 and :299).
  • The metadata path sets let encoding = 'UTF-8' and only changes it if readFile(absPath, 'utf-8') throws, which it never does for invalid UTF-8 (source/tools/read-file.tsx:81-92).

Additional Context

Proposed fix (happy to adjust): once #1866 is in, both can reuse what it adds.

  • Formatter: refuse a file that isn't valid UTF-8 with the same validateEditableEncoding() error before asking the language server for edits. formatFileWithLsp could read the file through the file cache (which now records notUtf8), or check buffer.isUtf8 on its own read.
  • Label: use the cache's notUtf8 flag, already in scope in that block, to report something like Not valid UTF-8 (legacy encoding?) instead of UTF-8.

For tests: formatting a Latin-1 file is refused and the bytes stay the same; a UTF-8 file with non-ASCII text still formats; and metadata_only on a Latin-1 file doesn't say UTF-8.

  • I have searched existing issues to ensure this is not a duplicate
  • I can reproduce this issue consistently
  • This issue occurs with the latest version of nanocoder

@addyCooks could you take a look when you have a moment? I'd like to work on this one too, so please assign it to me if that works for you.

Activity

  1. added
    needs-triageQueued for automated issue triage
    area:lspLanguage server integration
    area:toolsTool implementations and tool-calling
    bugSomething isn't working
    and removed
    needs-triageQueued for automated issue triage
    on Oct 20, 2026
  2. Parth-Jadhav-2004 commented on Oct 21, 2026

    @Parth-Jadhav-2004

    Can I take this? I'd read the file as a Buffer and validate it with a fatal TextDecoder, so lsp_format_document refuses non-UTF-8 files like string_replace does after #1866, and read_file metadata stops labelling them UTF-8. I'll add Latin-1 fixture tests for both and reuse the #1866 helper if that lands first.

  3. addyCooks commented on Oct 27, 2026

    @addyCooks
    Collaborator

    Thanks @diyaavirmani, confirmed on current main. lsp_format_document reads and writes with 'utf-8' (lsp-format-document.tsx:292/299), and the metadata path in read_file only changes the label if readFile throws, which it never does for invalid UTF-8. I reproduced both at unit level: a Latin-1 file formatted to 63 61 66 ef bf bd 0a 6e 65 77 0a with "1 edit applied", and metadata_only printed "Encoding: UTF-8".

    Your plan makes sense: once #1866 is in, refuse a non-UTF-8 file in the formatter with the same error, and use the notUtf8 flag for the label. Please add the three tests you listed, and wait for #1866 to merge first.

    Go ahead and open a PR. I'll assign this to you.

    @Parth-Jadhav-2004 thanks for the offer, but the reporter asked first, so this one is taken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

area:lspLanguage server integrationarea:toolsTool implementations and tool-callingbugSomething isn't workinggood first issueGood for newcomers

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions