Skip to content

Commit a6f9691

Browse files
code-yeongyucode-yeongyu
committed
fix(apply-patch): keep binary previews byte-safe
1 parent b4a300f commit a6f9691

6 files changed

Lines changed: 309 additions & 25 deletions

File tree

‎packages/coding-agent/src/core/extensions/builtin/gpt-apply-patch/apply.ts‎

Lines changed: 55 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { mkdir, readFile, rename, rm, unlink, writeFile } from "node:fs/promises";
1+
import { mkdir, rename, rm, unlink, writeFile } from "node:fs/promises";
22
import path from "node:path";
33
import { withFileMutationQueue } from "../../../tools/file-mutation-queue.ts";
44
import { ApplyPatchError } from "./errors.ts";
@@ -73,6 +73,18 @@ async function writeFileAtomic(
7373
}
7474
}
7575

76+
async function writeBinaryFileAtomic(absPath: string, content: Uint8Array): Promise<void> {
77+
const tempPath = `${absPath}.tmp.${process.pid}.${Math.random().toString(16).slice(2)}`;
78+
await writeFile(tempPath, content);
79+
try {
80+
await rename(tempPath, absPath);
81+
} catch (error) {
82+
if (!hasErrorCode(error, "EEXIST")) throw error;
83+
await unlink(absPath);
84+
await rename(tempPath, absPath);
85+
}
86+
}
87+
7688
export async function __testWriteFileAtomic(
7789
absPath: string,
7890
content: string,
@@ -103,29 +115,60 @@ async function applySingleHunk(
103115
}
104116

105117
if (hunk.type === "delete") {
106-
const oldContent = await readFile(absolutePath, "utf-8");
118+
const source = await readPatchFileSnapshot(absolutePath);
107119
const preview = buildPatchPreviewFile({
108120
hunk,
109-
source: { exists: true, content: oldContent },
121+
source,
110122
newContent: "",
111123
});
112124
await rm(absolutePath);
113125
return { summary: `delete: ${hunk.filePath}`, appliedFile: hunk.filePath, fuzz: 0, preview };
114126
}
115127

116-
const currentContent = await readFile(absolutePath, "utf-8");
128+
const source = await readPatchFileSnapshot(absolutePath);
129+
if (!source.exists) {
130+
const error = new Error(`ENOENT: no such file or directory, open '${absolutePath}'`) as NodeJS.ErrnoException;
131+
error.code = "ENOENT";
132+
throw error;
133+
}
134+
const absoluteMovePath = hunk.movePath ? resolvePatchPath(cwd, hunk.movePath) : undefined;
135+
const moveDestination =
136+
absoluteMovePath && absoluteMovePath !== absolutePath
137+
? await readPatchFileSnapshot(absoluteMovePath)
138+
: undefined;
139+
if (source.binary) {
140+
if (hunk.chunks.length > 0) {
141+
throw new Error(`apply_patch cannot apply text hunks to binary file: ${hunk.filePath}`);
142+
}
143+
if (!hunk.movePath || !absoluteMovePath || !source.bytes) {
144+
throw new Error(`apply_patch cannot update binary file without a move destination: ${hunk.filePath}`);
145+
}
146+
const preview = buildPatchPreviewFile({
147+
hunk,
148+
source,
149+
newContent: "",
150+
...(moveDestination ? { moveDestination } : {}),
151+
});
152+
await mkdir(path.dirname(absoluteMovePath), { recursive: true });
153+
await writeBinaryFileAtomic(absoluteMovePath, source.bytes);
154+
if (absoluteMovePath !== absolutePath) await rm(absolutePath);
155+
return {
156+
summary: `move: ${hunk.filePath} -> ${hunk.movePath}`,
157+
appliedFile: hunk.movePath,
158+
fuzz: 0,
159+
preview,
160+
};
161+
}
162+
117163
const chunkResult =
118164
hunk.chunks.length === 0
119-
? { content: currentContent, fuzz: 0 }
120-
: replaceChunks(currentContent, hunk.filePath, hunk.chunks);
165+
? { content: source.content, fuzz: 0 }
166+
: replaceChunks(source.content, hunk.filePath, hunk.chunks);
121167

122-
if (hunk.movePath) {
123-
const absoluteMovePath = resolvePatchPath(cwd, hunk.movePath);
124-
const moveDestination =
125-
absoluteMovePath === absolutePath ? undefined : await readPatchFileSnapshot(absoluteMovePath);
168+
if (hunk.movePath && absoluteMovePath) {
126169
const preview = buildPatchPreviewFile({
127170
hunk,
128-
source: { exists: true, content: currentContent },
171+
source,
129172
newContent: chunkResult.content,
130173
...(moveDestination ? { moveDestination } : {}),
131174
});
@@ -142,7 +185,7 @@ async function applySingleHunk(
142185

143186
const preview = buildPatchPreviewFile({
144187
hunk,
145-
source: { exists: true, content: currentContent },
188+
source,
146189
newContent: chunkResult.content,
147190
});
148191
await writeFileAtomic(absolutePath, chunkResult.content);

‎packages/coding-agent/src/core/extensions/builtin/gpt-apply-patch/changes.md‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,35 @@
11
# changes
22

3+
## Binary-safe patch previews (2026-08-05)
4+
5+
### What changed
6+
7+
- `preview.ts`: reads source and move-destination snapshots as bytes, classifies NUL-containing or invalid-UTF-8 files
8+
as binary before constructing any line or unified diff, and carries that typed marker through pending and completed
9+
previews.
10+
- `apply.ts`: deletion and update/move results reuse the same byte-aware snapshot. Move-only binary patches preserve
11+
the original bytes atomically, while text hunks against binary sources fail instead of decoding and rewriting them.
12+
- `preview-format.ts` / `types.ts`: binary files render as a concise `(binary)` summary with no diff, patch payload, or
13+
synthetic line counts.
14+
15+
### Why
16+
17+
Deleting an accidental PNG with `apply_patch` decoded the image as UTF-8, built a normal line diff containing `�PNG`,
18+
NUL/control bytes, `IHDR`, and `IDAT`, then rendered that payload inside the live TUI card. Character/line truncation
19+
bounded the size but did not make binary content safe.
20+
21+
### Why this belongs in the extension
22+
23+
The builtin owns the source snapshot, preview metadata, persisted result details, and custom renderer. Fixing the
24+
shared differential renderer would only hide one consumer while leaving poisoned binary diffs in session and
25+
app-server result data.
26+
27+
### Expected upstream conflict zones
28+
29+
- LOW: `preview.ts` snapshot decoding and binary preview construction.
30+
- LOW: `apply.ts` delete-source snapshot reuse.
31+
- LOW: `preview-format.ts` per-file summary formatting and `types.ts` preview metadata.
32+
333
## Codemode lazy activation (2026-08-04)
434

535
### What changed

‎packages/coding-agent/src/core/extensions/builtin/gpt-apply-patch/preview-format.ts‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,10 @@ function formatLineCountSummary(added: number, removed: number): string {
6868
return `(+${added} -${removed})`;
6969
}
7070

71+
function formatPatchFileSummary(file: ApplyPatchPreviewFile): string {
72+
return file.binary ? "(binary)" : formatLineCountSummary(file.added, file.removed);
73+
}
74+
7175
function countLines(text: string): number {
7276
if (text.length === 0) return 0;
7377
let lines = 1;
@@ -127,7 +131,7 @@ export function formatPatchPreview(
127131
const file = preview.files[0];
128132
if (file) {
129133
lines.push(
130-
`• ${formatPatchOperation(file.operation)} ${formatPatchFilePath(file, cwd)} ${formatLineCountSummary(file.added, file.removed)}`,
134+
`• ${formatPatchOperation(file.operation)} ${formatPatchFilePath(file, cwd)} ${formatPatchFileSummary(file)}`,
131135
);
132136
if (expanded && file.diff)
133137
lines.push(
@@ -142,7 +146,7 @@ export function formatPatchPreview(
142146
const noun = preview.files.length === 1 ? "file" : "files";
143147
lines.push(`• Edited ${preview.files.length} ${noun} ${formatLineCountSummary(preview.added, preview.removed)}`);
144148
for (const file of preview.files) {
145-
lines.push(` └ ${formatPatchFilePath(file, cwd)} ${formatLineCountSummary(file.added, file.removed)}`);
149+
lines.push(` └ ${formatPatchFilePath(file, cwd)} ${formatPatchFileSummary(file)}`);
146150
if (expanded && file.diff)
147151
lines.push(
148152
...truncatePreview(file.diff)
@@ -205,10 +209,10 @@ export function renderPatchPreview(
205209
if (expanded) {
206210
try {
207211
const renderFile = (file: ApplyPatchPreviewFile, headerPrefix: string): string => {
208-
const header = `• ${formatPatchOperation(file.operation)} ${formatPatchFilePath(file, cwd)} ${formatLineCountSummary(file.added, file.removed)}`;
212+
const header = `• ${formatPatchOperation(file.operation)} ${formatPatchFilePath(file, cwd)} ${formatPatchFileSummary(file)}`;
209213
if (!file.diff) {
210214
return headerPrefix.length > 0
211-
? `${headerPrefix}${formatPatchFilePath(file, cwd)} ${formatLineCountSummary(file.added, file.removed)}`
215+
? `${headerPrefix}${formatPatchFilePath(file, cwd)} ${formatPatchFileSummary(file)}`
212216
: header;
213217
}
214218

@@ -217,7 +221,7 @@ export function renderPatchPreview(
217221
theme,
218222
});
219223
if (headerPrefix.length > 0) {
220-
const nestedHeader = `${headerPrefix}${formatPatchFilePath(file, cwd)} ${formatLineCountSummary(file.added, file.removed)}`;
224+
const nestedHeader = `${headerPrefix}${formatPatchFilePath(file, cwd)} ${formatPatchFileSummary(file)}`;
221225
return `${nestedHeader}\n${renderedDiff
222226
.split("\n")
223227
.map((line) => ` ${line}`)

‎packages/coding-agent/src/core/extensions/builtin/gpt-apply-patch/preview.ts‎

Lines changed: 42 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,22 @@ import { resolvePatchPath } from "./workspace.ts";
1616
export type PatchFileSnapshot = {
1717
readonly exists: boolean;
1818
readonly content: string;
19+
readonly binary?: true;
20+
readonly bytes?: Uint8Array;
1921
};
2022

2123
export async function readPatchFileSnapshot(absolutePath: string): Promise<PatchFileSnapshot> {
2224
try {
23-
return { exists: true, content: await readFile(absolutePath, "utf-8") };
25+
const bytes = await readFile(absolutePath);
26+
if (bytes.includes(0)) return { exists: true, content: "", binary: true, bytes };
27+
try {
28+
return {
29+
exists: true,
30+
content: new TextDecoder("utf-8", { fatal: true }).decode(bytes),
31+
};
32+
} catch {
33+
return { exists: true, content: "", binary: true, bytes };
34+
}
2435
} catch (error) {
2536
if (error && typeof error === "object" && "code" in error && error.code === "ENOENT") {
2637
return { exists: false, content: "" };
@@ -57,6 +68,17 @@ export function buildPatchPreviewFile(input: {
5768
readonly moveDestination?: PatchFileSnapshot;
5869
}): ApplyPatchPreviewFile {
5970
const { hunk, source, newContent, moveDestination } = input;
71+
if (source.binary || moveDestination?.binary) {
72+
return {
73+
filePath: hunk.filePath,
74+
...(hunk.type === "update" && hunk.movePath ? { movePath: hunk.movePath } : {}),
75+
operation: hunk.type === "add" && source.exists ? "update" : hunk.type,
76+
binary: true,
77+
diff: "",
78+
added: 0,
79+
removed: 0,
80+
};
81+
}
6082
switch (hunk.type) {
6183
case "add": {
6284
const operation = source.exists ? "update" : "add";
@@ -102,24 +124,34 @@ async function createPatchPreviewFile(cwd: string, hunk: ParsedPatch): Promise<A
102124
return buildPatchPreviewFile({ hunk, source, newContent: hunk.content });
103125
}
104126
case "delete": {
105-
const oldContent = await readFile(absolutePath, "utf-8");
127+
const source = await readPatchFileSnapshot(absolutePath);
106128
return buildPatchPreviewFile({
107129
hunk,
108-
source: { exists: true, content: oldContent },
130+
source,
109131
newContent: "",
110132
});
111133
}
112134
case "update": {
113-
const oldContent = await readFile(absolutePath, "utf-8");
114-
const newContent =
115-
hunk.chunks.length === 0 ? oldContent : replaceChunks(oldContent, hunk.filePath, hunk.chunks).content;
135+
const source = await readPatchFileSnapshot(absolutePath);
116136
const moveDestination =
117137
hunk.movePath && hunk.movePath !== hunk.filePath
118138
? await readPatchFileSnapshot(resolvePatchPath(cwd, hunk.movePath))
119139
: undefined;
140+
if (source.binary || moveDestination?.binary) {
141+
return buildPatchPreviewFile({
142+
hunk,
143+
source,
144+
newContent: "",
145+
...(moveDestination ? { moveDestination } : {}),
146+
});
147+
}
148+
const newContent =
149+
hunk.chunks.length === 0
150+
? source.content
151+
: replaceChunks(source.content, hunk.filePath, hunk.chunks).content;
120152
return buildPatchPreviewFile({
121153
hunk,
122-
source: { exists: true, content: oldContent },
154+
source,
123155
newContent,
124156
...(moveDestination ? { moveDestination } : {}),
125157
});
@@ -132,7 +164,9 @@ async function createPatchPreviewFile(cwd: string, hunk: ParsedPatch): Promise<A
132164
}
133165

134166
function hasPreviewChange(file: ApplyPatchPreviewFile): boolean {
135-
return file.operation !== "update" || file.movePath !== undefined || file.diff.trim().length > 0;
167+
return (
168+
file.binary === true || file.operation !== "update" || file.movePath !== undefined || file.diff.trim().length > 0
169+
);
136170
}
137171

138172
export async function createPatchPreview(cwd: string, hunks: ParsedPatch[]): Promise<ApplyPatchPreview> {

‎packages/coding-agent/src/core/extensions/builtin/gpt-apply-patch/types.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ export type ApplyPatchPreviewFile = {
3838
filePath: string;
3939
movePath?: string;
4040
operation: ApplyPatchOperation;
41+
binary?: true;
4142
diff: string;
4243
patch?: string;
4344
added: number;

0 commit comments

Comments
 (0)