refactor(edit): remove auto-indentation and add line-ending normalization

- [x] Remove auto-indentation from text mode edits (caller controls whitespace)
- [x] Add line-ending detection and normalization for both AST and text modes
- [x] Share edit logic via new `spliceContent` function for both modes
- [x] Fix diff to emit "No newline at end of file" markers
- [x] Fix diff to strip raw CR from CRLF file output
- [x] Remove double-unescape of backslash sequences in new_content
- [x] Fix countDiffLines to be hunk-aware (correctly count lines starting with +/-)
- [x] Fix block-comment stripping to remove standalone lines cleanly
- [x] Fix Python license header stripping to preserve separator blank lines
This commit is contained in:
2026-05-29 00:17:36 +01:00
parent f1643e7b81
commit 9af2801b1b
9 changed files with 596 additions and 334 deletions
+18 -19
View File
@@ -11,16 +11,6 @@ import (
"github.com/mark3labs/mcp-go/mcp"
)
// unescapeNewlines converts literal \n, \t, \" sequences to actual characters.
// This handles cases where MCP clients send double-escaped JSON strings.
func unescapeNewlines(s string) string {
s = strings.ReplaceAll(s, "\\n", "\n")
s = strings.ReplaceAll(s, "\\t", "\t")
s = strings.ReplaceAll(s, "\\\"", "\"")
s = strings.ReplaceAll(s, "\\\\", "\\")
return s
}
// handleEditApply handles the edit_apply tool.
func (s *Server) handleEditApply(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
return s.handleEdit(ctx, request)
@@ -51,8 +41,12 @@ func (s *Server) handleEdit(ctx context.Context, request mcp.CallToolRequest) (*
return mcp.NewToolResultError("file is outside workspace root"), nil
}
// new_content arrives already fully decoded by the JSON-RPC layer (mcp-go):
// JSON escapes such as \n, \t, \\, \" have been resolved to their real bytes.
// It must therefore be used verbatim — any further unescaping would corrupt
// legitimate backslash sequences in source code (string literals, regexes,
// Windows paths). See TestEditApplyPreservesBackslashSequences*.
newContent := request.GetString("new_content", "")
newContent = unescapeNewlines(newContent)
selectorName := request.GetString("selector_name", "")
@@ -114,21 +108,26 @@ func (s *Server) handleEdit(ctx context.Context, request mcp.CallToolRequest) (*
}
}
// countDiffLines counts added (+) and removed (-) lines in a unified diff string.
// countDiffLines counts added (+) and removed (-) content lines in a unified diff.
// Only lines inside hunks are counted (everything after the first "@@" header), so the
// "---"/"+++" file headers are skipped structurally — and content whose own text starts
// with + or - is counted correctly rather than mistaken for a header. The git-style
// "\ No newline at end of file" marker is ignored.
func countDiffLines(diff string) (added, removed int) {
inHunk := false
for _, line := range strings.Split(diff, "\n") {
if len(line) == 0 {
if strings.HasPrefix(line, "@@") {
inHunk = true
continue
}
if !inHunk || line == "" {
continue
}
switch line[0] {
case '+':
if !strings.HasPrefix(line, "+++") {
added++
}
added++
case '-':
if !strings.HasPrefix(line, "---") {
removed++
}
removed++
}
}
return
+95 -39
View File
@@ -1,47 +1,103 @@
package server
import "testing"
import (
"context"
"os"
"strings"
"testing"
func TestUnescapeNewlines(t *testing.T) {
tests := []struct {
name string
input string
expected string
}{
{
name: "no escapes",
input: "func test() {\n\treturn true;\n}",
expected: "func test() {\n\treturn true;\n}",
},
{
name: "literal backslash n",
input: "func test() {\\n\\treturn true;\\n}",
expected: "func test() {\n\treturn true;\n}",
},
{
name: "literal backslash t",
input: "func test() {\\n\\treturn true;\\n}",
expected: "func test() {\n\treturn true;\n}",
},
{
name: "literal quotes",
input: `func test() {\n\treturn \"true\";\n}`,
expected: "func test() {\n\treturn \"true\";\n}",
},
"github.com/mark3labs/mcp-go/mcp"
)
{
name: "mixed content",
input: "line1\\nline2\\tindented\\nline3",
expected: "line1\nline2\tindented\nline3",
},
// callEdit invokes handleEditApply with the given args and returns the result.
func callEdit(t *testing.T, srv *Server, args map[string]any) *mcp.CallToolResult {
t.Helper()
req := mcp.CallToolRequest{}
req.Params.Arguments = args
res, err := srv.handleEditApply(context.Background(), req)
if err != nil {
t.Fatalf("handleEditApply error: %v", err)
}
if res == nil {
t.Fatal("handleEditApply returned nil result")
}
return res
}
// TestEditApplyPreservesBackslashSequencesText is the primary regression guard for the
// double-unescape bug: new_content arrives from the JSON-RPC layer already fully decoded,
// so the handler must write it to disk verbatim. Literal backslash sequences (\n, \t, \",
// \\) that legitimately appear in source code must survive byte-for-byte.
func TestEditApplyPreservesBackslashSequencesText(t *testing.T) {
tmpDir := t.TempDir()
srv := newTestServer(t, tmpDir)
f := writeFile(t, tmpDir, "note.txt", "PLACEHOLDER\n")
// Exactly the bytes a client intends after JSON decoding: backslash sequences are
// real source text here, NOT escapes to be interpreted.
newContent := `printf("a\nb\tc"); re = \d+; path = "C:\\tmp"; q = \"`
res := callEdit(t, srv, map[string]any{
"file": f,
"operation": "replace",
"selector_text": "PLACEHOLDER",
"new_content": newContent,
"response": "none",
})
if res.IsError {
t.Fatalf("edit returned error: %+v", res.Content)
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := unescapeNewlines(tt.input)
if result != tt.expected {
t.Errorf("unescapeNewlines(%q) = %q, expected %q", tt.input, result, tt.expected)
}
})
got, err := os.ReadFile(f)
if err != nil {
t.Fatalf("read back: %v", err)
}
want := newContent + "\n"
if string(got) != want {
t.Fatalf("new_content not written verbatim (double-unescape regression).\nwant: %q\ngot: %q", want, string(got))
}
}
// TestCountDiffLinesHunkAware verifies the line counter skips file headers structurally and
// counts content lines whose own text begins with + or - (previously dropped as "+++"/"---").
func TestCountDiffLinesHunkAware(t *testing.T) {
diff := "--- a.txt\n+++ a.txt\n@@ -1,3 +1,3 @@\n context\n+++plusprefix\n+normal add\n---minusprefix\n-normal del\n"
added, removed := countDiffLines(diff)
if added != 2 || removed != 2 {
t.Fatalf("hunk-aware diff count wrong: added=%d removed=%d (want 2 and 2)", added, removed)
}
}
// TestEditApplyPreservesBackslashSequencesCode covers the AST/code path: the same verbatim
// guarantee must hold for syntactically-validated code files.
func TestEditApplyPreservesBackslashSequencesCode(t *testing.T) {
tmpDir := t.TempDir()
srv := newTestServer(t, tmpDir)
src := "package main\n\nfunc demo() {\n\tprintln(\"old\")\n}\n"
f := writeFile(t, tmpDir, "demo.go", src)
// Real tab indentation + literal backslash sequences inside string/raw-string literals.
newBody := "func demo() {\n\tprintln(\"a\\nb\\tc\")\n\t_ = `\\d+`\n\t_ = \"C:\\\\tmp\"\n}"
res := callEdit(t, srv, map[string]any{
"file": f,
"operation": "replace",
"selector_kind": "function_declaration",
"selector_name": "demo",
"new_content": newBody,
"response": "none",
})
if res.IsError {
t.Fatalf("edit returned error: %+v", res.Content)
}
got, err := os.ReadFile(f)
if err != nil {
t.Fatalf("read back: %v", err)
}
if !strings.Contains(string(got), newBody) {
t.Fatalf("backslash sequences corrupted in code edit.\nwant substring:\n%q\ngot file:\n%q", newBody, string(got))
}
}
+2
View File
@@ -137,6 +137,8 @@ const helpEditApply = "# edit_apply — flags and examples\n\n" +
"| `none` | Empty response (silent success) |\n\n" +
"`compact_response=true` is a deprecated alias for `response=\"count\"` kept for pre-v2 compatibility.\n\n" +
"For code files (Go, TypeScript, JavaScript, Python, C, C++, Rust) syntax is validated before writing — the edit is rejected if it would produce invalid syntax.\n\n" +
"## new_content handling\n\n" +
"`new_content` is inserted **verbatim**. Send it as normal JSON — real newlines (or JSON `\\n`), with quotes JSON-escaped as usual. The server does not re-interpret escape sequences, so literal backslash sequences in source (`\\n`, `\\t`, `\\\\`, regexes, Windows paths) are written exactly as given. Indentation is not auto-applied — include the leading whitespace you want. Line endings are normalized to the file's existing convention (LF or CRLF).\n\n" +
"## Selector types\n\n" +
"### AST-mode selectors (code files)\n" +
"- `selector_kind` — AST node type (e.g. `function_declaration`, `class_declaration`)\n" +
+1 -1
View File
@@ -313,7 +313,7 @@ func (s *Server) registerTools() {
mcp.Description("Edit operation: replace, insert_before, insert_after, delete"),
),
mcp.WithString("new_content",
mcp.Description("New content (required for replace/insert operations)"),
mcp.Description("New content (required for replace/insert operations). Inserted verbatim: send it as normal JSON (real newlines, quotes JSON-escaped as usual). The server does NOT re-interpret escape sequences, so backslash sequences in source code (e.g. \\n, \\t, \\\\ inside string literals, regexes, or Windows paths) are preserved exactly. Indentation is not auto-applied — include the leading whitespace you want. Line endings are normalized to the file's existing convention (LF or CRLF)."),
),
// AST-mode selectors (for code files)
mcp.WithString("selector_kind",