Prechádzať zdrojové kódy

fix: correct rewrite ordering, empty-file handling and rollback fidelity in apply-patch

- Fold dependency groups in place only when no later hunk touches their
  paths; on interference emit the update standalone (original patch order
  preserved, move destination delete / source re-add sequences stay valid)
- Serialize accepted rescue hits as non-overlapping canonical chunks with a
  whole-file verification fallback, so shared context lines are not consumed
  twice on re-apply
- Keep Add File trailing empty lines distinct from the terminator (canonical
  newline-terminated contents across parse, fold, format and re-parse)
- Represent empty text as zero lines so EOF insertions on empty files do not
  gain a phantom leading blank line
- Store rollback snapshots as raw buffers to restore binary files
  byte-for-byte
- Track effective file modes sequentially so moves transfer permissions to
  later writes on the destination, and delete/recreate drops the stale mode
- Resolve workspace root and worktree lazily in the path guard (no orphaned
  promises), drop the unused realpath cache and redundant clone/branch logic
dhaern 1 týždeň pred
rodič
commit
51ad5a64f4

+ 1 - 1
src/hooks/apply-patch/codec.test.ts

@@ -37,7 +37,7 @@ PATCH`),
     expect(parsed.hunks[0]).toEqual({
       type: 'add',
       path: 'added.txt',
-      contents: 'alpha',
+      contents: 'alpha\n',
     });
     expect(parsed.hunks[1]).toEqual({ type: 'delete', path: 'removed.txt' });
     expect(parsed.hunks[2]).toEqual({

+ 12 - 2
src/hooks/apply-patch/codec.ts

@@ -169,7 +169,13 @@ function parseAdd(lines: string[], index: number, mode: ParseMode) {
     at += 1;
   }
 
-  return { content: contents.join('\n'), next: at };
+  // Canonical Add representation: either empty (no lines) or newline-
+  // terminated. `+a` followed by `+` must describe two lines ("a\n\n"),
+  // not collapse into one; a lone `+` is a single empty line ("\n").
+  return {
+    content: contents.length === 0 ? '' : `${contents.join('\n')}\n`,
+    next: at,
+  };
 }
 
 function parsePatchInternal(patchText: string, mode: ParseMode): ParsedPatch {
@@ -320,7 +326,11 @@ function renderAddContents(contents: string): string[] {
     return [];
   }
 
-  return contents.split('\n').map((line) => `+${line}`);
+  // Canonical contents are newline-terminated; the trailing empty element
+  // produced by split is the terminator, not an extra empty `+` line.
+  const lines = contents.split('\n');
+  lines.pop();
+  return lines.map((line) => `+${line}`);
 }
 
 export function formatPatch(patch: ParsedPatch): string {

+ 20 - 28
src/hooks/apply-patch/execution-context.ts

@@ -18,9 +18,10 @@ import type {
 } from './types';
 
 type PathGuardContext = {
-  rootReal: Promise<string>;
+  root: string;
+  rootReal?: Promise<string>;
+  worktree?: string;
   worktreeReal?: Promise<string>;
-  realCache: Map<string, Promise<string>>;
 };
 
 type FileCacheContext = {
@@ -107,44 +108,35 @@ function createPathGuardContext(
   root: string,
   worktree: string | undefined,
 ): PathGuardContext {
-  return {
-    rootReal: real(root),
-    worktreeReal: worktree && worktree !== '/' ? real(worktree) : undefined,
-    realCache: new Map(),
-  };
-}
-
-async function realCached(
-  ctx: PathGuardContext,
-  target: string,
-): Promise<string> {
-  const resolvedTarget = path.resolve(target);
-  let pending = ctx.realCache.get(resolvedTarget);
-  if (!pending) {
-    pending = real(resolvedTarget);
-    ctx.realCache.set(resolvedTarget, pending);
-  }
-
-  return await pending;
+  return { root, worktree };
 }
 
 async function guard(ctx: PathGuardContext, target: string): Promise<void> {
-  const [targetReal, rootReal] = await Promise.all([
-    realCached(ctx, target),
-    ctx.rootReal,
-  ]);
-  if (inside(rootReal, targetReal)) {
+  const targetReal = await real(target);
+  // Both resolutions are lazy: whichever rejects first is observed here,
+  // and the other is never created, so no promise is left unhandled.
+  ctx.rootReal ??= real(ctx.root);
+  if (inside(await ctx.rootReal, targetReal)) {
     return;
   }
 
+  if (!ctx.worktree) {
+    throw createApplyPatchBlockedError(
+      `patch contains path outside workspace root: ${target}`,
+    );
+  }
+
+  // Resolve the worktree lazily: patches whose targets all live inside root
+  // never pay for it, and its rejection stays observed inside this flow
+  // instead of becoming an unhandled promise.
+  ctx.worktreeReal ??= ctx.worktree !== '/' ? real(ctx.worktree) : undefined;
   if (!ctx.worktreeReal) {
     throw createApplyPatchBlockedError(
       `patch contains path outside workspace root: ${target}`,
     );
   }
 
-  const treeReal = await ctx.worktreeReal;
-  if (inside(treeReal, targetReal)) {
+  if (inside(await ctx.worktreeReal, targetReal)) {
     return;
   }
 

+ 2 - 2
src/hooks/apply-patch/hook.test.ts

@@ -650,7 +650,7 @@ garbage
     expect(rewritten.hunks[0]).toEqual({
       type: 'add',
       path: 'added.txt',
-      contents: 'fresh',
+      contents: 'fresh\n',
     });
     expect(rewritten.hunks[1]).toEqual({
       type: 'update',
@@ -729,7 +729,7 @@ garbage
     expect(parsePatch(output.args.patchText as string).hunks[0]).toMatchObject({
       type: 'add',
       path: '../shared.txt',
-      contents: 'fresh',
+      contents: 'fresh\n',
     });
   });
 

+ 276 - 6
src/hooks/apply-patch/operations.test.ts

@@ -1,8 +1,15 @@
 import { describe, expect, test } from 'bun:test';
-import { chmod, mkdir, stat, symlink } from 'node:fs/promises';
+import {
+  chmod,
+  mkdir,
+  readFile,
+  stat,
+  symlink,
+  writeFile,
+} from 'node:fs/promises';
 import path from 'node:path';
 
-import { parsePatch } from './codec';
+import { formatPatch, parsePatch } from './codec';
 import {
   isApplyPatchBlockedError,
   isApplyPatchValidationError,
@@ -408,7 +415,7 @@ garbage
     expect(parsePatch(rewritten.patchText).hunks[0]).toMatchObject({
       type: 'add',
       path: 'added.txt',
-      contents: 'fresh',
+      contents: 'fresh\n',
     });
 
     await expect(
@@ -518,7 +525,7 @@ garbage
     expect(parsePatch(rewritten.patchText).hunks[0]).toMatchObject({
       type: 'add',
       path: '../shared.txt',
-      contents: 'fresh',
+      contents: 'fresh\n',
     });
 
     await expect(
@@ -902,7 +909,7 @@ garbage
       {
         type: 'add',
         path: 'added.txt',
-        contents: 'alpha\nBETA',
+        contents: 'alpha\nBETA\n',
       },
     ]);
   });
@@ -931,7 +938,7 @@ garbage
       {
         type: 'add',
         path: 'nested/after.txt',
-        contents: 'alpha\nBETA',
+        contents: 'alpha\nBETA\n',
       },
     ]);
   });
@@ -1525,4 +1532,267 @@ garbage
       'apply_patch blocked: patch contains path outside workspace root:',
     );
   });
+
+  test('parsePatch/formatPatch keep an Add File trailing empty line distinct from the terminator', () => {
+    const patchText = `*** Begin Patch
+*** Add File: trailing.txt
++a
++
+*** End Patch`;
+
+    const parsed = parsePatch(patchText);
+    expect(parsed.hunks[0]).toMatchObject({
+      type: 'add',
+      contents: 'a\n\n',
+    });
+
+    // Round-trip preserves the trailing empty `+` line.
+    const reformatted = formatPatch(parsed);
+    expect(reformatted).toContain('+a\n+\n');
+    expect(parsePatch(reformatted).hunks[0]).toEqual(parsed.hunks[0]);
+  });
+
+  test('parsePatch represents an empty Add File as empty contents', () => {
+    const parsed = parsePatch(`*** Begin Patch
+*** Add File: empty.txt
+*** End Patch`);
+
+    expect(parsed.hunks[0]).toMatchObject({
+      type: 'add',
+      contents: '',
+    });
+  });
+
+  test('rewritePatch re-emits a folded move after a later delete of its destination', async () => {
+    const root = await createTempDir();
+    await writeFixture(root, 'a.txt', 'alpha\n');
+    await writeFixture(root, 'b.txt', 'beta\n');
+
+    const result = await rewritePatch(
+      root,
+      `*** Begin Patch
+*** Update File: a.txt
+@@
+-alpha
++ALPHA
+*** Update File: b.txt
+@@
+-beta
++BETA
+*** Delete File: b.txt
+*** Update File: a.txt
+*** Move to: b.txt
+@@
+-ALPHA
++ALPHA-MOVED
+*** End Patch`,
+      DEFAULT_OPTIONS,
+    );
+
+    // The folded update+move over a.txt must be emitted after Delete b.txt
+    // so the destination is free, not hoisted above it.
+    const hunks = parsePatch(result.patchText).hunks;
+    const deleteIndex = hunks.findIndex((hunk) => hunk.type === 'delete');
+    const lastIndex = hunks.length - 1;
+    expect(deleteIndex).toBeGreaterThanOrEqual(0);
+    expect(hunks[lastIndex]).toMatchObject({
+      type: 'update',
+      path: 'a.txt',
+      move_path: 'b.txt',
+    });
+    expect(deleteIndex).toBeLessThan(lastIndex);
+
+    await applyPatch(root, result.patchText);
+    expect(await readText(root, 'b.txt')).toBe('ALPHA-MOVED\n');
+  });
+
+  test('rewritePatch serializes non-overlapping canonical chunks when a rescue shares context', async () => {
+    const root = await createTempDir();
+    await writeFixture(root, 'sample.txt', 'a\nold\nz\n');
+
+    const result = await rewritePatch(
+      root,
+      `*** Begin Patch
+*** Update File: sample.txt
+@@
+ a
+-stale
++fresh
+ z
+@@
+-z
++Z2
+*** End Patch`,
+      DEFAULT_OPTIONS,
+    );
+
+    // The stale first chunk is rescued with shared suffix `z`; the second
+    // chunk edits that same `z`. The rewritten patch must not consume `z`
+    // twice and must re-apply cleanly.
+    await applyPatch(root, result.patchText);
+    expect(await readText(root, 'sample.txt')).toBe('a\nfresh\nZ2\n');
+  });
+
+  test('EOF insertion into an empty file does not create a leading blank line', async () => {
+    const root = await createTempDir();
+    await writeFixture(root, 'empty.txt', '');
+
+    await applyPatch(
+      root,
+      `*** Begin Patch
+*** Update File: empty.txt
+@@
++hello
+*** End of File
+*** End Patch`,
+      DEFAULT_OPTIONS,
+    );
+
+    // Zero-line representation: no phantom leading blank line. The missing
+    // final newline matches the empty file's original terminator state.
+    expect(await readText(root, 'empty.txt')).toBe('hello');
+  });
+
+  test('applyPreparedChanges rollback restores binary files byte-for-byte', async () => {
+    const root = await createTempDir();
+    const binaryPath = path.join(root, 'bin.dat');
+    const original = Buffer.from([0xff, 0xfe, 0x00, 0x41, 0xff]);
+    await writeFile(binaryPath, original);
+    // Existing file at `blocker` makes the nested add fail with ENOTDIR.
+    await writeFile(path.join(root, 'blocker'), 'not-a-dir\n');
+
+    await expect(
+      applyPreparedChanges([
+        {
+          type: 'delete',
+          file: binaryPath,
+        },
+        {
+          type: 'add',
+          file: path.join(root, 'blocker', 'nested.txt'),
+          text: 'never written\n',
+        },
+      ]),
+    ).rejects.toThrow();
+
+    expect(Buffer.compare(await readFile(binaryPath), original)).toBe(0);
+  });
+
+  test('applyPreparedChanges transfers the source mode to later writes on the move destination', async () => {
+    const root = await createTempDir();
+    const source = path.join(root, 'src.txt');
+    await writeFile(source, 'one\n');
+    await chmod(source, 0o755);
+
+    await applyPreparedChanges([
+      {
+        type: 'update',
+        file: source,
+        move: path.join(root, 'dst.txt'),
+        text: 'ONE\n',
+      },
+      {
+        type: 'update',
+        file: path.join(root, 'dst.txt'),
+        text: 'TWO\n',
+      },
+    ]);
+
+    expect(await readText(root, 'dst.txt')).toBe('TWO\n');
+    expect((await stat(path.join(root, 'dst.txt'))).mode & 0o777).toBe(0o755);
+  });
+
+  test('applyPreparedChanges drops the stale mode when a move destination is deleted and recreated', async () => {
+    const root = await createTempDir();
+    const source = path.join(root, 'src.txt');
+    const dst = path.join(root, 'dst.txt');
+    await writeFile(source, 'one\n');
+    await chmod(source, 0o755);
+
+    await applyPreparedChanges([
+      {
+        type: 'update',
+        file: source,
+        move: dst,
+        text: 'ONE\n',
+      },
+      { type: 'delete', file: dst },
+      { type: 'add', file: dst, text: 'fresh\n' },
+      { type: 'update', file: dst, text: 'FRESH\n' },
+    ]);
+
+    const dstStat = await stat(dst).catch(() => null);
+    expect(dstStat).not.toBeNull();
+    // The recreated-and-updated file must not inherit the moved source's
+    // execute bits through the trailing update.
+    expect((dstStat?.mode ?? 0o777) & 0o111).toBe(0);
+    expect(await readFile(dst, 'utf-8')).toBe('FRESH\n');
+  });
+
+  test('rewritePatch emits a folded add with canonical terminator when finalText lacks a final newline', async () => {
+    const root = await createTempDir();
+
+    // Empty Add (no `+` lines) followed by an EOF update on the empty file:
+    // finalText is 'hello' with NO final newline, which the renderer would
+    // silently drop without canonical termination.
+    const result = await rewritePatch(
+      root,
+      `*** Begin Patch
+*** Add File: made.txt
+*** Update File: made.txt
+@@
++hello
+*** End of File
+*** End Patch`,
+      DEFAULT_OPTIONS,
+    );
+
+    const hunks = parsePatch(result.patchText).hunks;
+    expect(hunks[hunks.length - 1]).toMatchObject({
+      type: 'add',
+      contents: 'hello\n',
+    });
+
+    await applyPatch(root, result.patchText);
+    expect(await readText(root, 'made.txt')).toBe('hello\n');
+  });
+
+  test('rewritePatch keeps a later add on the freed move source after the folded group', async () => {
+    const root = await createTempDir();
+    await writeFixture(root, 'a.txt', 'alpha\n');
+
+    const result = await rewritePatch(
+      root,
+      `*** Begin Patch
+*** Update File: a.txt
+*** Move to: b.txt
+@@
+-alpha
++ALPHA
+*** Add File: a.txt
++recreated
+*** Update File: b.txt
+@@
+-ALPHA
++ALPHA-MOVED
+*** End Patch`,
+      DEFAULT_OPTIONS,
+    );
+
+    // The add on the freed source a.txt must stay AFTER the folded move
+    // group, or it would collide with the still-existing original a.txt.
+    const hunks = parsePatch(result.patchText).hunks;
+    const addIndex = hunks.findIndex(
+      (hunk) => hunk.type === 'add' && hunk.path === 'a.txt',
+    );
+    const moveIndex = hunks.findIndex(
+      (hunk) => hunk.type === 'update' && hunk.move_path === 'b.txt',
+    );
+    expect(moveIndex).toBeGreaterThanOrEqual(0);
+    expect(addIndex).toBeGreaterThan(moveIndex);
+
+    await applyPatch(root, result.patchText);
+    expect(await readText(root, 'a.txt')).toBe('recreated\n');
+    expect(await readText(root, 'b.txt')).toBe('ALPHA-MOVED\n');
+  });
 });

+ 29 - 11
src/hooks/apply-patch/prepared-changes.ts

@@ -180,7 +180,9 @@ type FileSnapshot =
   | { type: 'missing' }
   | {
       type: 'file';
-      text: string;
+      // Raw bytes: rolling back must restore binary files byte-for-byte,
+      // which utf-8 decoding cannot guarantee.
+      bytes: Buffer;
       mode: number;
     };
 
@@ -195,7 +197,7 @@ async function readSnapshot(filePath: string): Promise<FileSnapshot> {
 
     return {
       type: 'file',
-      text: await fs.readFile(filePath, 'utf-8'),
+      bytes: await fs.readFile(filePath),
       mode: stat.mode & 0o7777,
     };
   } catch (error) {
@@ -220,7 +222,7 @@ async function restoreSnapshot(
   }
 
   await fs.mkdir(path.dirname(filePath), { recursive: true });
-  await writeFileAtomically(filePath, snapshot.text, snapshot.mode);
+  await writeFileAtomically(filePath, snapshot.bytes, snapshot.mode);
 }
 
 function createTempSiblingPath(target: string): string {
@@ -232,14 +234,14 @@ function createTempSiblingPath(target: string): string {
 
 async function writeFileAtomically(
   target: string,
-  text: string,
+  data: string | Buffer,
   mode?: number,
 ): Promise<void> {
   const tempPath = createTempSiblingPath(target);
 
   try {
     await fs.mkdir(path.dirname(target), { recursive: true });
-    await fs.writeFile(tempPath, text, 'utf-8');
+    await fs.writeFile(tempPath, data);
     if (mode !== undefined) {
       await fs.chmod(tempPath, mode);
     }
@@ -352,32 +354,48 @@ export async function applyPreparedChanges(
 
   assertPreparedApplyPreconditions(changes, snapshots);
 
+  // Effective mode per path as changes are applied sequentially: a move
+  // transfers the source mode to the destination, so later writes on that
+  // destination must keep it instead of falling back to the initial
+  // snapshot (which may not exist yet).
+  const effectiveModes = new Map<string, number | undefined>();
+  function effectiveMode(filePath: string): number | undefined {
+    if (effectiveModes.has(filePath)) {
+      return effectiveModes.get(filePath);
+    }
+
+    return getSnapshotMode(snapshots.get(filePath) ?? { type: 'missing' });
+  }
+
   try {
     for (const change of changes) {
       if (change.type === 'add') {
         await writeFileAtomically(change.file, change.text);
+        // A (re)created path has no prior mode to preserve: drop any mode
+        // tracked for it so later writes do not resurrect a stale one.
+        effectiveModes.set(change.file, undefined);
         continue;
       }
 
       if (change.type === 'delete') {
         await fs.unlink(change.file);
+        effectiveModes.set(change.file, undefined);
         continue;
       }
 
       if (change.move && change.move !== change.file) {
-        await writeFileAtomically(
-          change.move,
-          change.text,
-          getSnapshotMode(snapshots.get(change.file) ?? { type: 'missing' }),
-        );
+        const mode = effectiveMode(change.file);
+        await writeFileAtomically(change.move, change.text, mode);
         await fs.unlink(change.file);
+        effectiveModes.set(change.move, mode);
+        effectiveModes.set(change.file, undefined);
         continue;
       }
 
       await writeFileAtomically(
         change.file,
         change.text,
-        getSnapshotMode(snapshots.get(change.file) ?? { type: 'missing' }),
+        effectiveMode(change.file),
       );
     }
   } catch (error) {

+ 15 - 2
src/hooks/apply-patch/resolution.ts

@@ -29,8 +29,9 @@ function splitFileLines(text: string): FileLines {
   const eol = text.match(/\r\n|\n|\r/)?.[0] === '\r\n' ? '\r\n' : '\n';
   const normalized = text.replace(/\r\n/g, '\n').replace(/\r/g, '\n');
   const hasFinalNewline = normalized.endsWith('\n');
-  const lines = normalized.split('\n');
-  if (hasFinalNewline) {
+  // Empty text is zero lines, not one empty line; '\n' is one empty line.
+  const lines = normalized.length === 0 ? [] : normalized.split('\n');
+  if (lines.length > 0 && hasFinalNewline) {
     lines.pop();
   }
 
@@ -149,6 +150,8 @@ export function locateChunk(
       rewritten,
       strategy: undefined,
       matchComparator: match.comparator,
+      canonical_start: match.index,
+      canonical_end: match.index + canonical_old_lines.length,
     };
   }
 
@@ -178,6 +181,8 @@ export function locateChunk(
         rewritten: true,
         strategy: 'prefix/suffix',
         matchComparator: 'exact',
+        canonical_start: canonicalStart,
+        canonical_end: canonicalEnd,
       };
     }
   }
@@ -205,6 +210,8 @@ export function locateChunk(
         rewritten: true,
         strategy: 'lcs',
         matchComparator: 'exact',
+        canonical_start: rescued.hit.start,
+        canonical_end: rescued.hit.start + rescued.hit.del,
       };
     }
   }
@@ -268,6 +275,8 @@ function resolveUpdateChunksFromFileLines(
           rewritten: false,
           strategy,
           matchComparator: 'exact',
+          canonical_start: lines.length,
+          canonical_end: lines.length,
         });
         start = lines.length;
         continue;
@@ -312,6 +321,8 @@ function resolveUpdateChunksFromFileLines(
           rewritten: !anchorMatch.exact,
           strategy: anchorMatch.exact ? strategy : 'anchor',
           matchComparator: anchorMatch.comparator,
+          canonical_start: insertAt,
+          canonical_end: insertAt,
         });
         start = insertAt;
         continue;
@@ -336,6 +347,8 @@ function resolveUpdateChunksFromFileLines(
         rewritten: true,
         strategy,
         matchComparator: anchorMatch.comparator,
+        canonical_start: insertAt,
+        canonical_end: insertAt + 1,
       });
       start = insertAt;
       continue;

+ 137 - 33
src/hooks/apply-patch/rewrite.ts

@@ -49,6 +49,11 @@ function normalizeTextLineEndings(text: string): string {
 }
 
 function splitPatchTextLines(text: string): string[] {
+  // Empty text is zero lines, not one empty line; '\n' is one empty line.
+  if (text.length === 0) {
+    return [];
+  }
+
   const normalized = normalizeTextLineEndings(text);
   const lines = normalized.split('\n');
   if (normalized.endsWith('\n')) {
@@ -116,15 +121,6 @@ function clonePatchChunks(
 }
 
 function minimizeMergedChunk(chunk: UpdatePatchHunk['chunks'][number]) {
-  if (chunk.old_lines.length === 0 && chunk.new_lines.length === 0) {
-    return {
-      old_lines: [],
-      new_lines: [],
-      change_context: chunk.change_context,
-      is_end_of_file: chunk.is_end_of_file,
-    };
-  }
-
   let prefixLength = 0;
   while (
     prefixLength < chunk.old_lines.length &&
@@ -195,9 +191,11 @@ function mergeSameFileUpdateGroupChunks(
     return undefined;
   }
 
+  // minimizeMergedChunk never mutates its input, so the original chunk
+  // arrays can be mapped directly.
   const mergedChunks = [
-    ...clonePatchChunks(group.chunks).map(minimizeMergedChunk),
-    ...clonePatchChunks(nextChunks).map(minimizeMergedChunk),
+    ...group.chunks.map(minimizeMergedChunk),
+    ...nextChunks.map(minimizeMergedChunk),
   ];
 
   try {
@@ -214,10 +212,6 @@ function mergeSameFileUpdateGroupChunks(
   }
 }
 
-function addContentsFromFinalText(text: string): string {
-  return text.endsWith('\n') ? text.slice(0, -1) : text;
-}
-
 function renderRewriteDependencyGroup(
   group: RewriteDependencyGroup,
   cfg: ApplyPatchRuntimeOptions,
@@ -226,7 +220,10 @@ function renderRewriteDependencyGroup(
     return {
       type: 'add',
       path: group.group.outputPath,
-      contents: addContentsFromFinalText(group.group.finalText),
+      // Guarantee the canonical newline-terminated Add representation:
+      // finalText may legitimately lack a final newline (e.g. updates on a
+      // no-final-newline file), which the renderer would otherwise drop.
+      contents: stageAddedText(group.group.finalText),
     };
   }
 
@@ -320,6 +317,37 @@ export async function rewritePatch(
       dependencyGroups.delete(filePath);
     }
 
+    function hunkTouchedPaths(hunk: PatchHunk): Set<string> {
+      const touched = new Set<string>([path.resolve(root, hunk.path)]);
+      if (hunk.type === 'update' && hunk.move_path) {
+        touched.add(path.resolve(root, hunk.move_path));
+      }
+      return touched;
+    }
+
+    // Fold a dependency group in place only when no hunk emitted after it
+    // touches its paths: reordering around interleaved hunks (delete of the
+    // move destination, add recreating the move source) is exactly where
+    // folded patches stop being order-equivalent. On any interference the
+    // fold is abandoned and the caller emits the update standalone, which
+    // preserves the original patch ordering and is always safe.
+    function reemitFoldedGroup(
+      groupIndex: number,
+      rendered: PatchHunk,
+    ): number | undefined {
+      const touched = hunkTouchedPaths(rendered);
+      for (let index = groupIndex + 1; index < rewritten.length; index += 1) {
+        for (const target of hunkTouchedPaths(rewritten[index])) {
+          if (touched.has(target)) {
+            return undefined;
+          }
+        }
+      }
+
+      rewritten[groupIndex] = rendered;
+      return groupIndex;
+    }
+
     for (const hunk of hunks) {
       if (hunk.type === 'add') {
         const filePath = path.resolve(root, hunk.path);
@@ -376,16 +404,82 @@ export async function rewritePatch(
         cfg,
       );
 
-      const next = resolved.map((chunk, index) => ({
-        old_lines: [...chunk.canonical_old_lines],
-        new_lines: [...chunk.canonical_new_lines],
-        change_context:
-          chunk.canonical_change_context ?? hunk.chunks[index].change_context,
-        is_end_of_file:
+      let next: UpdatePatchHunk['chunks'] = [];
+      let lastCanonicalEnd = -1;
+      let sawCanonicalOverlap = false;
+      for (const [index, chunk] of resolved.entries()) {
+        const changeContext =
+          chunk.canonical_change_context ?? hunk.chunks[index].change_context;
+        const isEndOfFile =
           hunk.chunks[index].is_end_of_file && chunk.resolved_is_end_of_file
             ? true
-            : undefined,
-      }));
+            : undefined;
+
+        const previous = next[next.length - 1];
+        const overlap = previous ? lastCanonicalEnd - chunk.canonical_start : 0;
+
+        if (
+          previous &&
+          overlap > 0 &&
+          overlap <= previous.old_lines.length &&
+          chunk.canonical_old_lines.length >= overlap
+        ) {
+          // A rescue extended this chunk's canonical range over lines the
+          // previous chunk already claimed. Serialize both as one chunk so
+          // every source line is consumed exactly once; separate chunks
+          // would re-match consumed context and fail on re-apply.
+          previous.old_lines = previous.old_lines
+            .slice(0, previous.old_lines.length - overlap)
+            .concat(chunk.canonical_old_lines);
+          previous.new_lines = previous.new_lines
+            .slice(0, previous.new_lines.length - overlap)
+            .concat(chunk.canonical_new_lines);
+          previous.is_end_of_file = isEndOfFile ?? previous.is_end_of_file;
+          lastCanonicalEnd = Math.max(lastCanonicalEnd, chunk.canonical_end);
+          sawCanonicalOverlap = true;
+          continue;
+        }
+
+        if (overlap > 0) {
+          sawCanonicalOverlap = true;
+        }
+
+        next.push({
+          old_lines: [...chunk.canonical_old_lines],
+          new_lines: [...chunk.canonical_new_lines],
+          change_context: changeContext,
+          is_end_of_file: isEndOfFile,
+        });
+        lastCanonicalEnd = chunk.canonical_end;
+      }
+
+      if (sawCanonicalOverlap) {
+        // Overlap merges must reproduce the accepted hits exactly. If an
+        // exotic shape does not, fall back to a verified whole-file chunk
+        // instead of shipping a rewrite that cannot re-apply.
+        try {
+          if (
+            deriveNewContentFromText(filePath, current.text, next, cfg) !==
+            nextText
+          ) {
+            next = createCollapsedUpdateHunk(
+              hunk.path,
+              filePath,
+              current.text,
+              nextText,
+              cfg,
+            ).chunks;
+          }
+        } catch {
+          next = createCollapsedUpdateHunk(
+            hunk.path,
+            filePath,
+            current.text,
+            nextText,
+            cfg,
+          ).chunks;
+        }
+      }
 
       for (const chunk of resolved) {
         if (!chunk.rewritten) {
@@ -397,6 +491,7 @@ export async function rewritePatch(
       const nextOutputPath = hunk.move_path ?? hunk.path;
       const nextOutputFilePath = movePath ?? filePath;
 
+      let folded = false;
       if (current.derived && currentDependency) {
         const nextGroup = combineDependentUpdateGroup(
           filePath,
@@ -407,17 +502,26 @@ export async function rewritePatch(
           nextOutputFilePath,
           cfg,
         );
-        rewritten[currentDependency.group.index] = renderRewriteDependencyGroup(
-          nextGroup,
-          cfg,
+        const foldedIndex = reemitFoldedGroup(
+          currentDependency.group.index,
+          renderRewriteDependencyGroup(nextGroup, cfg),
         );
-        changed = true;
-        clearDependencyGroup(filePath);
-        if (movePath && movePath !== filePath) {
-          clearDependencyGroup(movePath);
+        if (foldedIndex !== undefined) {
+          changed = true;
+          clearDependencyGroup(filePath);
+          if (movePath && movePath !== filePath) {
+            clearDependencyGroup(movePath);
+          }
+          nextGroup.group.index = foldedIndex;
+          dependencyGroups.set(nextOutputFilePath, nextGroup);
+          folded = true;
         }
-        dependencyGroups.set(nextOutputFilePath, nextGroup);
-      } else {
+      }
+
+      if (!folded) {
+        // First touch of this path, or an interfering hunk made the fold
+        // order-unsafe: emit this update standalone, which preserves the
+        // original patch ordering.
         rewritten.push(createUpdateHunk(hunk.path, next, hunk.move_path));
         clearDependencyGroup(filePath);
         if (movePath && movePath !== filePath) {

+ 5 - 0
src/hooks/apply-patch/types.ts

@@ -101,6 +101,11 @@ export type ResolvedChunk = {
   rewritten: boolean;
   strategy?: ApplyPatchRescueStrategy;
   matchComparator?: MatchComparatorName;
+  // Half-open [canonical_start, canonical_end) range of the source lines
+  // covered by the canonical representation. Used to keep serialized chunks
+  // non-overlapping when rescue extends a chunk over shared context lines.
+  canonical_start: number;
+  canonical_end: number;
 };
 
 export type RescueResult =