Skip to content

Commit 889a09f

Browse files
yyq1025claude
andcommitted
fix Edit/Write diff sheet: git-style patch headers + self-resetting boundary
Every Edit/Write detail crashed the iOS tool-call sheet since the Pierre cutover (ba4b254): makeUnifiedDiff emitted bare @@ hunks, Pierre's parsePatchFiles found zero files in them, and PatchDiff's getSingularPatch threw — then the render boundary inside the RESIDENT webview stuck on that error for every subsequent open. The working-tree path never crashed because git-watch synthesizes full git headers. makeUnifiedDiff now emits the git form (diff --git + --- a/ + +++ b/, basename only — no /Users/... leak, drives language inference). Git mode is load-bearing beyond the obvious: non-git mode splits files on ^---\s+\S anywhere, so a deleted '-- comment' line (SQL/Lua/Haskell) renders as '--- comment' and forges a phantom second file. Hunk-body lines always carry a +/-/space/\ prefix, so nothing can forge a diff --git boundary. Also pads both fragment sides with a final newline to suppress jsdiff's '\ No newline at end of file' markers — they assert a file-end fact that snippet diffs cannot know. App side: the hand-rolled RenderBoundary becomes react-error-boundary with resetKeys=[payload], so a bad payload only poisons its own open. Bump @pierre/diffs 1.2.7 -> 1.2.10 (worker asset re-synced): upstream independently fixed the SQL-comment parse in 1.2.9 and 1.2.10 allows shiki 4. Edge-case sweep (frontmatter ---, editing .patch files, CRLF, CJK, 2000-line writes, spaced filenames) verified against the new parser. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 6b522cb commit 889a09f

5 files changed

Lines changed: 275 additions & 137 deletions

File tree

packages/app/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
"@nandorojo/galeria": "^3.0.3",
2323
"@noble/ed25519": "^3.1.0",
2424
"@noble/hashes": "^2.2.0",
25-
"@pierre/diffs": "^1.2.7",
25+
"@pierre/diffs": "^1.2.10",
2626
"@pierre/theme": "^1.0.3",
2727
"@shikijs/core": "^4.2.0",
2828
"@shikijs/langs": "^4.2.0",
@@ -57,6 +57,7 @@
5757
"marked": "^18.0.5",
5858
"react": "19.2.3",
5959
"react-dom": "19.2.3",
60+
"react-error-boundary": "^6.1.2",
6061
"react-native": "0.85.3",
6162
"react-native-drawer-layout": "^4.2.5",
6263
"react-native-enriched-markdown": "0.6.0",

packages/app/src/components/transcript/pierre-view.tsx

Lines changed: 27 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,8 @@ import {
99
WorkerPoolContextProvider,
1010
} from "@pierre/diffs/react";
1111
import type { DOMProps } from "expo/dom";
12-
import {
13-
Component,
14-
type ReactNode,
15-
useCallback,
16-
useEffect,
17-
useMemo,
18-
useRef,
19-
useState,
20-
} from "react";
12+
import { useCallback, useEffect, useMemo, useRef, useState } from "react";
13+
import { ErrorBoundary } from "react-error-boundary";
2114
import { JETBRAINS_MONO_CSS } from "./jetbrains-mono-font";
2215

2316
/**
@@ -109,33 +102,25 @@ export interface PierreViewProps {
109102
dom?: DOMProps;
110103
}
111104

112-
/** Surfaces a render-phase throw instead of a silent blank webview. */
113-
class RenderBoundary extends Component<
114-
{ children: ReactNode },
115-
{ err: string | null }
116-
> {
117-
state = { err: null as string | null };
118-
static getDerivedStateFromError(e: unknown) {
119-
return { err: e instanceof Error ? e.message : String(e) };
120-
}
121-
render() {
122-
if (this.state.err) {
123-
return (
124-
<pre
125-
style={{
126-
color: "#dc2626",
127-
whiteSpace: "pre-wrap",
128-
wordBreak: "break-word",
129-
font: "11px ui-monospace, monospace",
130-
padding: 8,
131-
}}
132-
>
133-
{this.state.err}
134-
</pre>
135-
);
136-
}
137-
return this.props.children;
138-
}
105+
/** Fallback for the render boundary: surfaces a render-phase throw instead
106+
* of a silent blank webview. Hosted by react-error-boundary's
107+
* <ErrorBoundary> with `resetKeys=[decoded]` — the boundary lives inside
108+
* the RESIDENT webview, so without that reset one bad payload would leave
109+
* every subsequent sheet open stuck on the same error. */
110+
function RenderErrorFallback({ error }: { error: unknown }) {
111+
return (
112+
<pre
113+
style={{
114+
color: "#dc2626",
115+
whiteSpace: "pre-wrap",
116+
wordBreak: "break-word",
117+
font: "11px ui-monospace, monospace",
118+
padding: 8,
119+
}}
120+
>
121+
{error instanceof Error ? error.message : String(error)}
122+
</pre>
123+
);
139124
}
140125

141126
export default function PierreView({
@@ -202,7 +187,7 @@ export default function PierreView({
202187
);
203188

204189
// Backstop per content change: if Pierre never commits (e.g. it throws before
205-
// first render and RenderBoundary takes over), onPostRender never fires — open
190+
// first render and the error boundary takes over), onPostRender never fires — open
206191
// anyway after a timeout so the sheet can't hang. Empty content is ready now.
207192
useEffect(() => {
208193
if (decoded.length === 0) {
@@ -299,7 +284,10 @@ export default function PierreView({
299284
);
300285

301286
const tree = (
302-
<RenderBoundary>
287+
<ErrorBoundary
288+
FallbackComponent={RenderErrorFallback}
289+
resetKeys={[decoded]}
290+
>
303291
{kind === "diff" ? (
304292
// Single-file diff scrolls the webview document. With WKWebView's auto
305293
// content-inset disabled (contentInsetAdjustmentBehavior:"never"), pad
@@ -367,7 +355,7 @@ export default function PierreView({
367355
/>
368356
</div>
369357
)}
370-
</RenderBoundary>
358+
</ErrorBoundary>
371359
);
372360

373361
return (

packages/daemon/src/messages/normalize.test.ts

Lines changed: 89 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ describe("normalize", () => {
198198
expect(item.summary).toBe("foo.ts");
199199
});
200200

201-
it("Edit unifiedDiff is hunk-only (`@@ ... @@\\n+/-/space lines`) with no Index/===/---/+++ preamble", () => {
201+
it("Edit unifiedDiff carries git-style basename headers (Pierre needs them to detect the file)", () => {
202202
const out = normalize([
203203
assistantMsg("a-1", [
204204
{
@@ -216,11 +216,96 @@ describe("normalize", () => {
216216
const item = out[0];
217217
if (item?.type !== "tool_call") throw new Error("expected tool_call");
218218
if (item.detail.type !== "edit") throw new Error("expected edit detail");
219-
expect(item.detail.unifiedDiff.startsWith("@@")).toBe(true);
219+
// A hunks-only diff parses as 0 files in Pierre and PatchDiff throws
220+
// (the iOS sheet crash of 2026-06-12). The GIT header form is required
221+
// (see makeUnifiedDiff docblock); basename keeps /Users/... out of the
222+
// rendered header and drives language inference.
223+
expect(
224+
item.detail.unifiedDiff.startsWith(
225+
"diff --git a/foo.ts b/foo.ts\n--- a/foo.ts\n+++ b/foo.ts\n@@",
226+
),
227+
).toBe(true);
220228
expect(item.detail.unifiedDiff).not.toContain("Index:");
221229
expect(item.detail.unifiedDiff).not.toContain("===");
222-
expect(item.detail.unifiedDiff).not.toContain("--- ");
223-
expect(item.detail.unifiedDiff).not.toContain("+++ ");
230+
expect(item.detail.unifiedDiff).not.toContain("/abs/");
231+
});
232+
233+
it("deleting a '-- comment' line cannot forge a Pierre file boundary (git headers)", () => {
234+
// In Pierre's NON-git mode, files split on `^---\s+\S` anywhere — and a
235+
// deleted SQL/Lua/Haskell comment line `-- old` renders as `--- old` in
236+
// the hunk body, forging a phantom second file (getSingularPatch throws).
237+
// Git mode splits only on `diff --git` lines; hunk-body lines always
238+
// carry a +/-/space/\ prefix, so content can never forge that.
239+
const out = normalize([
240+
assistantMsg("a-1", [
241+
{
242+
type: "tool_use",
243+
id: "tu-1",
244+
name: "Edit",
245+
input: {
246+
file_path: "/abs/query.sql",
247+
old_string: "SELECT 1;\n-- old comment\nSELECT 2;",
248+
new_string: "SELECT 1;\nSELECT 2;",
249+
},
250+
},
251+
]),
252+
]);
253+
const item = out[0];
254+
if (item?.type !== "tool_call") throw new Error("expected tool_call");
255+
if (item.detail.type !== "edit") throw new Error("expected edit detail");
256+
expect(item.detail.unifiedDiff.startsWith("diff --git ")).toBe(true);
257+
// The deleted comment is one hunk line — and must NOT be preceded by a
258+
// newline-`---`-space sequence parseable as a file header in git mode.
259+
expect(item.detail.unifiedDiff).toContain("\n--- old comment");
260+
expect((item.detail.unifiedDiff.match(/^diff --git /gm) ?? []).length).toBe(
261+
1,
262+
);
263+
});
264+
265+
it("Edit unifiedDiff has no '\\ No newline' markers (snippets say nothing about the file end)", () => {
266+
const out = normalize([
267+
assistantMsg("a-1", [
268+
{
269+
type: "tool_use",
270+
id: "tu-1",
271+
name: "Edit",
272+
input: {
273+
file_path: "/tmp/probe.md",
274+
// Neither side ends with \n — raw jsdiff would emit the marker
275+
// for both; the daemon pads a final newline to suppress it.
276+
old_string: "hello sidecode",
277+
new_string: "hello sidecode world",
278+
},
279+
},
280+
]),
281+
]);
282+
const item = out[0];
283+
if (item?.type !== "tool_call") throw new Error("expected tool_call");
284+
if (item.detail.type !== "edit") throw new Error("expected edit detail");
285+
expect(item.detail.unifiedDiff).toBe(
286+
"diff --git a/probe.md b/probe.md\n--- a/probe.md\n+++ b/probe.md\n@@ -1,1 +1,1 @@\n-hello sidecode\n+hello sidecode world",
287+
);
288+
});
289+
290+
it("Edit with old === new produces an empty diff (no headers-only shell)", () => {
291+
const out = normalize([
292+
assistantMsg("a-1", [
293+
{
294+
type: "tool_use",
295+
id: "tu-1",
296+
name: "Edit",
297+
input: {
298+
file_path: "/abs/foo.ts",
299+
old_string: "same",
300+
new_string: "same",
301+
},
302+
},
303+
]),
304+
]);
305+
const item = out[0];
306+
if (item?.type !== "tool_call") throw new Error("expected tool_call");
307+
if (item.detail.type !== "edit") throw new Error("expected edit detail");
308+
expect(item.detail.unifiedDiff).toBe("");
224309
});
225310

226311
it("Write produces an all-add diff against empty (we lack prior content)", () => {

packages/daemon/src/messages/tool-detail.ts

Lines changed: 53 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -418,38 +418,74 @@ function unknownDetail(name: string, rawInput: unknown): ToolCallDetail {
418418
}
419419

420420
/**
421-
* Compose a clean unified-diff string of just `@@...@@` hunks, ready for
422-
* MarkdownView's auto-detect diff renderer (no fence needed iOS-side).
421+
* Compose a git-style unified diff (`diff --git` + `---`/`+++` headers +
422+
* `@@` hunks), shaped for Pierre's PatchDiff (iOS tool-call sheet).
423+
*
424+
* The headers are REQUIRED, not cosmetic: Pierre's `parsePatchFiles`
425+
* needs file boundaries — a hunks-only string parses as metadata with
426+
* ZERO files, and `getSingularPatch` (inside `<PatchDiff>`) throws "must
427+
* contain exactly 1 file diff". This was exactly the iOS sheet crash on
428+
* every Edit/Write detail (found 2026-06-12).
429+
*
430+
* The GIT form specifically (not plain `--- x`/`+++ x` headers) is
431+
* load-bearing too: in non-git mode Pierre splits files on `^---\s+\S`
432+
* ANYWHERE in the patch, and a deleted content line that itself starts
433+
* with `-- ` (SQL/Lua/Haskell comments) renders as `--- foo` in the hunk
434+
* body — a phantom second file → same throw. Git mode only splits on
435+
* `diff --git` lines, and hunk-body lines always carry a +/-/space/\\
436+
* prefix, so no content can forge that boundary. Mirrors git-watch's
437+
* synthesizeAddPatch, which is why the working-tree path never crashed.
438+
*
439+
* Basename (not the absolute path) in all three header lines — drives
440+
* shiki language inference by extension and avoids leaking `/Users/...`
441+
* into the rendered header. Verified against @pierre/diffs@1.2.7:
442+
* `--- a/<name>` / `+++ b/<name>` parse via FILENAME_HEADER_REGEX_GIT
443+
* (prefixes stripped, spaces in names fine).
423444
*
424445
* We use jsdiff's `structuredPatch` rather than `createPatch` to skip the
425-
* full-file boilerplate (`Index:`, `===`, `--- file`, `+++ file`) that:
426-
* - CommonMark would otherwise mis-parse (`Index: foo\n====` becomes a
427-
* setext h1; `+`-prefixed lines become bullet lists)
428-
* - is cosmetic noise — MarkdownView's diff render already shows the
429-
* filename in the chip / accordion summary; repeating the path in a
430-
* `--- /Users/.../file` row above every diff is redundant.
446+
* `Index:` / `===` boilerplate Pierre doesn't need.
431447
*
432448
* Output for a small Edit:
449+
* diff --git a/foo.ts b/foo.ts
450+
* --- a/foo.ts
451+
* +++ b/foo.ts
433452
* @@ -1,3 +1,3 @@
434453
* -const x = 1;
435454
* +const x = 2;
436455
* const y = 3;
437456
*
438-
* Empty oldString === newString produces an empty string (no hunks).
457+
* Empty oldString === newString produces an empty string (no hunks — the
458+
* sheet shows "(no output)" instead of an empty diff shell).
439459
*/
440460
function makeUnifiedDiff(
441461
filePath: string,
442462
oldString: string,
443463
newString: string,
444464
): string {
445-
const patch = structuredPatch(filePath, filePath, oldString, newString);
446-
return patch.hunks
447-
.map(
448-
(h) =>
449-
`@@ -${h.oldStart},${h.oldLines} +${h.newStart},${h.newLines} @@\n` +
450-
h.lines.join("\n"),
451-
)
452-
.join("\n");
465+
// Suppress jsdiff's "\ No newline at end of file" markers by padding a
466+
// final newline onto both sides (line content is unchanged). The marker
467+
// asserts something about the FILE end, but Edit diffs compare file
468+
// FRAGMENTS — a snippet ending without \n says nothing about the file —
469+
// so the marker is noise stated as fact (and Pierre renders it as a
470+
// dedicated row). Claude's own UIs drop it too. "" stays "" (Write's
471+
// create-from-empty diff must keep the empty side truly empty).
472+
const a =
473+
oldString === "" || oldString.endsWith("\n") ? oldString : `${oldString}\n`;
474+
const b =
475+
newString === "" || newString.endsWith("\n") ? newString : `${newString}\n`;
476+
const patch = structuredPatch(filePath, filePath, a, b);
477+
if (patch.hunks.length === 0) return "";
478+
const name = basenameOf(filePath);
479+
return (
480+
`diff --git a/${name} b/${name}\n--- a/${name}\n+++ b/${name}\n` +
481+
patch.hunks
482+
.map(
483+
(h) =>
484+
`@@ -${h.oldStart},${h.oldLines} +${h.newStart},${h.newLines} @@\n` +
485+
h.lines.join("\n"),
486+
)
487+
.join("\n")
488+
);
453489
}
454490

455491
/**

0 commit comments

Comments
 (0)