fix(commit): preserve binary patch terminators in split-commit round-trip
parseFileDiffs split the captured `git diff --cached --binary` on " diff --git ", consuming the newline that terminates each file block, and patch.join ended with `.replace(/\n+$/, "")`. Both dropped the blank line that terminates a `GIT binary patch` block, so rebuilding a split-commit patch produced a corrupt binary patch rejected by `git apply --binary`. Split on a line-start lookahead so blocks keep their terminators verbatim, and concatenate join parts without stripping trailing newlines. Both trailing and mid-diff binary blocks now round-trip byte-exact. Fixes #8899
This commit is contained in:
@@ -12,6 +12,7 @@
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed `omp commit` split-commit failing with `corrupt binary patch` when a split commit contains a binary file. `parseFileDiffs` split the captured diff on `"\ndiff --git "`, consuming the `\n` that terminates each block, and `patch.join` stripped trailing newlines — both dropped the blank line that terminates a `GIT binary patch` block, so the rebuilt patch was rejected by `git apply --binary`. Both trailing and mid-diff binary blocks now survive the parse/rebuild round-trip byte-exact ([#8899](https://github.com/can1357/oh-my-pi/issues/8899)).
|
||||
- Fixed Claude Code marketplace plugins ignoring the `enabledPlugins` switch in `~/.claude/settings.json` and `.claude/settings(.local).json`: a plugin turned off for a project no longer loads there, and a local-scope install enabled for a project loads even when its recorded `projectPath` is a different directory
|
||||
- Fixed task and eval subagents discovering newly added agent definitions while resolving their role aliases from stale startup settings. Subagent preflight now atomically reloads persisted settings before agent discovery while preserving live runtime overrides.
|
||||
- Fixed images returned by tools mounted under `xd://` rendering only as file links instead of inline terminal graphics.
|
||||
|
||||
@@ -21,9 +21,13 @@ export function parseNumstat(output: string): NumstatEntry[] {
|
||||
|
||||
export function parseFileDiffs(diff: string): FileDiff[] {
|
||||
const sections: FileDiff[] = [];
|
||||
const parts = diff.split("\ndiff --git ");
|
||||
// Split on a line-start lookahead so each block keeps its terminating
|
||||
// newline(s) verbatim. Consuming the delimiter would drop the blank line
|
||||
// that terminates a `GIT binary patch` block, corrupting binary diffs on
|
||||
// rebuild (issue #8899).
|
||||
const parts = diff.split(/^(?=diff --git )/m);
|
||||
for (let index = 0; index < parts.length; index += 1) {
|
||||
const part = index === 0 ? parts[index] : `diff --git ${parts[index]}`;
|
||||
const part = parts[index];
|
||||
if (!part.trim()) continue;
|
||||
const lines = part.split("\n");
|
||||
const header = lines[0] ?? "";
|
||||
|
||||
@@ -2005,12 +2005,17 @@ export const patch = {
|
||||
}
|
||||
},
|
||||
|
||||
/** Join patch parts into a single patch string. */
|
||||
/**
|
||||
* Join patch parts into a single patch string.
|
||||
*
|
||||
* Each part is terminated with a single `\n` if it lacks one, then parts are
|
||||
* concatenated verbatim — matching git's native multi-file diff layout. Parts
|
||||
* are NOT separated by an extra blank line and trailing newlines are NOT
|
||||
* stripped: a `GIT binary patch` block ends in a blank line that
|
||||
* `git apply --binary` requires, and stripping it corrupts the patch (#8899).
|
||||
*/
|
||||
join(parts: string[]): string {
|
||||
return `${parts
|
||||
.map(part => (part.endsWith("\n") ? part : `${part}\n`))
|
||||
.join("\n")
|
||||
.replace(/\n+$/, "")}\n`;
|
||||
return parts.map(part => (part.endsWith("\n") ? part : `${part}\n`)).join("");
|
||||
},
|
||||
};
|
||||
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { describe, expect, test } from "bun:test";
|
||||
import { parseFileDiffs } from "@oh-my-pi/pi-coding-agent/commit/git/diff";
|
||||
import { patch } from "@oh-my-pi/pi-coding-agent/utils/git";
|
||||
|
||||
describe("joinPatch", () => {
|
||||
@@ -39,3 +40,31 @@ describe("joinPatch", () => {
|
||||
expect(result.endsWith("line2\n")).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("parseFileDiffs + patch.join binary round-trip", () => {
|
||||
// git's `GIT binary patch` block is terminated by a blank line that
|
||||
// `git apply --binary` requires. parseFileDiffs → patch.join must preserve
|
||||
// it byte-exact whether or not the binary block is the last file (#8899).
|
||||
const textBlock = "diff --git a/a.txt b/a.txt\n" + "--- a/a.txt\n+++ b/a.txt\n@@ -1,2 +1,2 @@\n a\n-b\n+b changed\n";
|
||||
const binaryBlock =
|
||||
"diff --git a/bin.dat b/bin.dat\n" +
|
||||
"index 1111111..2222222 100644\n" +
|
||||
"GIT binary patch\n" +
|
||||
"literal 6\n" +
|
||||
"zc$@zAB0000\n" +
|
||||
"\n";
|
||||
|
||||
test("preserves binary terminator when binary block is last", () => {
|
||||
const diff = textBlock + binaryBlock;
|
||||
const rebuilt = patch.join(parseFileDiffs(diff).map(f => f.content));
|
||||
expect(rebuilt).toBe(diff);
|
||||
expect(rebuilt.endsWith("zc$@zAB0000\n\n")).toBe(true);
|
||||
});
|
||||
|
||||
test("preserves binary terminator when binary block is not last", () => {
|
||||
const diff = binaryBlock + textBlock;
|
||||
const rebuilt = patch.join(parseFileDiffs(diff).map(f => f.content));
|
||||
expect(rebuilt).toBe(diff);
|
||||
expect(rebuilt.includes("zc$@zAB0000\n\ndiff --git a/a.txt")).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user