Skip to content

Commit 28a73fd

Browse files
committed
fix: harden apply_patch and clean up debug logging
- Normalize CRLF/CR to LF in patch parser before parsing (Windows compat) - Normalize CRLF/CR in file content before line matching - Strip UTF-8 BOM before patch application to prevent silent match failures - Add diagnostic logging when fuzzy match falls back (rstrip/trim/unicode) - Add 13 targeted tests: CRLF, mixed line endings, bare CR, Unicode paths, smart quotes, BOM stripping, whitespace fuzzy matching, unmatched lines - Replace debug console.log/error with structured Log calls in websearch.ts https://claude.ai/code/session_01A8LkdCir3TS3uCbx9L8CAb
1 parent 7927771 commit 28a73fd

3 files changed

Lines changed: 201 additions & 7 deletions

File tree

packages/opencode/src/patch/index.ts

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,8 @@ export namespace Patch {
188188
}
189189

190190
export function parsePatch(patchText: string): { hunks: Hunk[] } {
191-
const cleaned = stripHeredoc(patchText.trim())
191+
// Normalize all line endings to LF before parsing (handles CRLF/CR from Windows)
192+
const cleaned = stripHeredoc(patchText.replace(/\r\n/g, "\n").replace(/\r/g, "\n").trim())
192193
const lines = cleaned.split("\n")
193194
const hunks: Hunk[] = []
194195
let i = 0
@@ -317,6 +318,14 @@ export namespace Patch {
317318
throw new Error(`Failed to read file ${filePath}: ${error}`)
318319
}
319320

321+
// Strip UTF-8 BOM if present
322+
if (originalContent.charCodeAt(0) === 0xfeff) {
323+
originalContent = originalContent.slice(1)
324+
}
325+
326+
// Normalize CRLF to LF for consistent line matching
327+
originalContent = originalContent.replace(/\r\n/g, "\n").replace(/\r/g, "\n")
328+
320329
let originalLines = originalContent.split("\n")
321330

322331
// Drop trailing empty element for consistent line counting
@@ -470,11 +479,17 @@ export namespace Patch {
470479

471480
// Pass 2: rstrip (trim trailing whitespace)
472481
const rstrip = tryMatch(lines, pattern, startIndex, (a, b) => a.trimEnd() === b.trimEnd(), eof)
473-
if (rstrip !== -1) return rstrip
482+
if (rstrip !== -1) {
483+
log.info("fuzzy match: trailing whitespace difference", { line: rstrip, pass: "rstrip" })
484+
return rstrip
485+
}
474486

475487
// Pass 3: trim (both ends)
476488
const trim = tryMatch(lines, pattern, startIndex, (a, b) => a.trim() === b.trim(), eof)
477-
if (trim !== -1) return trim
489+
if (trim !== -1) {
490+
log.info("fuzzy match: leading/trailing whitespace difference", { line: trim, pass: "trim" })
491+
return trim
492+
}
478493

479494
// Pass 4: normalized (Unicode punctuation to ASCII)
480495
const normalized = tryMatch(
@@ -484,6 +499,9 @@ export namespace Patch {
484499
(a, b) => normalizeUnicode(a.trim()) === normalizeUnicode(b.trim()),
485500
eof,
486501
)
502+
if (normalized !== -1) {
503+
log.info("fuzzy match: Unicode normalization applied", { line: normalized, pass: "unicode" })
504+
}
487505
return normalized
488506
}
489507

packages/opencode/src/tool/websearch.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ import { Effect, Layer } from "effect"
66
import z from "zod"
77
import DISCRIPTION from "./websearch.txt"
88
import { Search } from "@/search"
9+
import { Log } from "../util/log"
10+
11+
const log = Log.create({ service: "websearch" })
912

1013
export const WebSearchTool = Tool.define("websearch", async () => {
1114
const searchPromise = Search.Default()
@@ -33,15 +36,15 @@ export const WebSearchTool = Tool.define("websearch", async () => {
3336
const results = await runPromise((tq) => tq.query(params.query, params.numResults, "STRUCTURAL"))
3437

3538
if (results.length > 0 && results[0].score > 0.85) {
36-
console.log(`[WebSearch] Cache hit for: ${params.query}`)
39+
log.info("cache hit", { query: params.query })
3740
return {
3841
output: `[FROM LOCAL CACHE] ${results[0].content}`,
3942
title: `Web search (cached): ${params.query}`,
4043
metadata: { cached: true, source: "turboquant", provider: "local", error: "" },
4144
}
4245
}
4346
} catch (e) {
44-
console.error("[WebSearch] Failed to query TurboQuant:", e)
47+
log.error("failed to query TurboQuant", { error: String(e) })
4548
}
4649

4750
const search = await searchPromise
@@ -53,7 +56,7 @@ export const WebSearchTool = Tool.define("websearch", async () => {
5356
}
5457
}
5558

56-
console.log(`[WebSearch] Searching for: ${params.query}`)
59+
log.info("searching", { query: params.query })
5760

5861
const searchResult = await search.search(params.query, {
5962
limit: params.numResults,
@@ -65,7 +68,7 @@ export const WebSearchTool = Tool.define("websearch", async () => {
6568
const sessionID = ctx.sessionID ?? "none"
6669
await runPromise((tq) => tq.store(params.query, searchResult!, { source: "websearch", sessionID }, "STRUCTURAL"))
6770
} catch (e) {
68-
console.error("[WebSearch] Failed to store in TurboQuant:", e)
71+
log.error("failed to store in TurboQuant", { error: String(e) })
6972
}
7073

7174
return {
Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
import { describe, expect, test } from "bun:test"
2+
import { Patch } from "../../src/patch"
3+
import path from "path"
4+
import fs from "fs"
5+
import { tmpdir } from "os"
6+
7+
describe("Patch parser edge cases", () => {
8+
test("parsePatch handles CRLF line endings in patch text", () => {
9+
const patchText =
10+
"*** Begin Patch\r\n*** Add File: test.txt\r\n+hello world\r\n*** End Patch\r\n"
11+
const result = Patch.parsePatch(patchText)
12+
expect(result.hunks).toHaveLength(1)
13+
expect(result.hunks[0].type).toBe("add")
14+
if (result.hunks[0].type === "add") {
15+
expect(result.hunks[0].contents).toBe("hello world")
16+
}
17+
})
18+
19+
test("parsePatch handles mixed CRLF and LF", () => {
20+
const patchText =
21+
"*** Begin Patch\r\n*** Add File: mixed.txt\n+line one\r\n+line two\n*** End Patch\n"
22+
const result = Patch.parsePatch(patchText)
23+
expect(result.hunks).toHaveLength(1)
24+
if (result.hunks[0].type === "add") {
25+
expect(result.hunks[0].contents).toContain("line one")
26+
expect(result.hunks[0].contents).toContain("line two")
27+
}
28+
})
29+
30+
test("parsePatch handles bare CR line endings", () => {
31+
const patchText =
32+
"*** Begin Patch\r*** Add File: cr.txt\r+content\r*** End Patch\r"
33+
const result = Patch.parsePatch(patchText)
34+
expect(result.hunks).toHaveLength(1)
35+
})
36+
37+
test("parsePatch returns empty hunks for empty patch", () => {
38+
const result = Patch.parsePatch("*** Begin Patch\n*** End Patch")
39+
expect(result.hunks).toHaveLength(0)
40+
})
41+
42+
test("parsePatch handles Unicode in file paths", () => {
43+
const patchText = [
44+
"*** Begin Patch",
45+
"*** Add File: src/données.txt",
46+
"+contenu français",
47+
"*** End Patch",
48+
].join("\n")
49+
const result = Patch.parsePatch(patchText)
50+
expect(result.hunks).toHaveLength(1)
51+
if (result.hunks[0].type === "add") {
52+
expect(result.hunks[0].path).toBe("src/données.txt")
53+
expect(result.hunks[0].contents).toBe("contenu français")
54+
}
55+
})
56+
57+
test("parsePatch handles Unicode smart quotes in content", () => {
58+
const patchText = [
59+
"*** Begin Patch",
60+
'*** Add File: quotes.txt',
61+
'+He said \u201CHello\u201D',
62+
"*** End Patch",
63+
].join("\n")
64+
const result = Patch.parsePatch(patchText)
65+
expect(result.hunks).toHaveLength(1)
66+
if (result.hunks[0].type === "add") {
67+
expect(result.hunks[0].contents).toContain("\u201C")
68+
}
69+
})
70+
71+
test("parsePatch handles delete with CRLF", () => {
72+
const patchText = "*** Begin Patch\r\n*** Delete File: old.txt\r\n*** End Patch\r\n"
73+
const result = Patch.parsePatch(patchText)
74+
expect(result.hunks).toHaveLength(1)
75+
expect(result.hunks[0].type).toBe("delete")
76+
expect(result.hunks[0].path).toBe("old.txt")
77+
})
78+
79+
test("parsePatch handles update with move and CRLF", () => {
80+
const patchText = [
81+
"*** Begin Patch",
82+
"*** Update File: src/old.ts",
83+
"*** Move to: src/new.ts",
84+
"@@ context line",
85+
" unchanged",
86+
"-old line",
87+
"+new line",
88+
"*** End Patch",
89+
].join("\r\n")
90+
const result = Patch.parsePatch(patchText)
91+
expect(result.hunks).toHaveLength(1)
92+
const hunk = result.hunks[0]
93+
expect(hunk.type).toBe("update")
94+
if (hunk.type === "update") {
95+
expect(hunk.path).toBe("src/old.ts")
96+
expect(hunk.move_path).toBe("src/new.ts")
97+
expect(hunk.chunks).toHaveLength(1)
98+
expect(hunk.chunks[0].old_lines).toContain("old line")
99+
expect(hunk.chunks[0].new_lines).toContain("new line")
100+
}
101+
})
102+
})
103+
104+
describe("deriveNewContentsFromChunks edge cases", () => {
105+
const tmpDir = path.join(tmpdir(), `patch-test-${process.pid}`)
106+
107+
function writeTemp(name: string, content: string): string {
108+
const filePath = path.join(tmpDir, name)
109+
fs.mkdirSync(path.dirname(filePath), { recursive: true })
110+
fs.writeFileSync(filePath, content)
111+
return filePath
112+
}
113+
114+
test("handles file with CRLF line endings", () => {
115+
const filePath = writeTemp("crlf.txt", "line one\r\nline two\r\nline three\r\n")
116+
const result = Patch.deriveNewContentsFromChunks(filePath, [
117+
{
118+
old_lines: ["line two"],
119+
new_lines: ["line TWO"],
120+
},
121+
])
122+
expect(result.content).toContain("line TWO")
123+
expect(result.content).not.toContain("\r")
124+
})
125+
126+
test("handles file with UTF-8 BOM", () => {
127+
const filePath = writeTemp("bom.txt", "\uFEFFline one\nline two\n")
128+
const result = Patch.deriveNewContentsFromChunks(filePath, [
129+
{
130+
old_lines: ["line one"],
131+
new_lines: ["LINE ONE"],
132+
},
133+
])
134+
expect(result.content).toContain("LINE ONE")
135+
// BOM should be stripped, not affect matching
136+
expect(result.content.charCodeAt(0)).not.toBe(0xfeff)
137+
})
138+
139+
test("handles whitespace-only differences via fuzzy matching", () => {
140+
const filePath = writeTemp("whitespace.txt", " indented\n also indented\n")
141+
const result = Patch.deriveNewContentsFromChunks(filePath, [
142+
{
143+
old_lines: ["indented"], // no leading spaces in patch
144+
new_lines: ["REPLACED"],
145+
},
146+
])
147+
expect(result.content).toContain("REPLACED")
148+
})
149+
150+
test("handles Unicode smart quotes via normalization", () => {
151+
// File has ASCII quotes, patch has smart quotes
152+
const filePath = writeTemp("unicode-quotes.txt", 'He said "hello"\nGoodbye\n')
153+
const result = Patch.deriveNewContentsFromChunks(filePath, [
154+
{
155+
old_lines: ['He said \u201Chello\u201D'], // smart quotes
156+
new_lines: ['He said "hi"'],
157+
},
158+
])
159+
expect(result.content).toContain('He said "hi"')
160+
})
161+
162+
test("throws on unmatched lines", () => {
163+
const filePath = writeTemp("nomatch.txt", "alpha\nbeta\ngamma\n")
164+
expect(() =>
165+
Patch.deriveNewContentsFromChunks(filePath, [
166+
{
167+
old_lines: ["nonexistent line"],
168+
new_lines: ["replacement"],
169+
},
170+
]),
171+
).toThrow(/Failed to find expected lines/)
172+
})
173+
})

0 commit comments

Comments
 (0)