Merge PR #8993: fix(commit): preserve binary patch terminators in split-commit round-trip (@roboomp)

packages/coding-agent/CHANGELOG.md

@@ -16,6 +16,7 @@
 - Fixed the `/btw` panel re-committing its frame to native scrollback on every update while the primary turn is still streaming: a live region that pins itself (an anchored HUD/panel such as `/btw`) no longer leaks its scrolled-off rows just because an unpinned transcript seam sits above it in the same frame ([#8793](https://github.com/can1357/oh-my-pi/issues/8793)).
 - Fixed a submitted `/skill:<name>` command staying invisible in the transcript until its awaited preflight (memory recall, `before_agent_start` hooks, auto-thinking classification, pre-prompt compaction) finished, so a slow step such as a Hindsight auto-recall timeout made the command look unaccepted. Idle skill submissions now paint an optimistic row immediately — like a normal prompt — and reconcile it in place when the canonical `message_start` lands ([#8895](https://github.com/can1357/oh-my-pi/issues/8895)).
 - Fixed broker-backed MCP OAuth credentials never refreshing, so remote OAuth MCP servers dropped out of `/mcp` once their access token expired under `omp auth-broker serve`. The client threw on the broker-redacted refresh sentinel instead of asking the broker to refresh, and the broker had no `mcp_oauth:*` refresh path (`POST /v1/credential/:id/refresh` answered `Unknown OAuth provider`). The client now routes redacted MCP refreshes through the broker, and the broker refreshes MCP credentials with a generic `refresh_token` grant from the credential's embedded token endpoint and client id — so the background refresher also keeps MCP tokens live ([#8933](https://github.com/can1357/oh-my-pi/issues/8933)).
+- 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 revived subagents (warm lifecycle reviver and cold persisted reviver) rebuilding the session without initializing the extension runtime, leaving every runtime action throwing `ExtensionRuntimeNotInitializedError`. An extension with a `tool_call` handler that touched a runtime action (e.g. `appendEntry`) then tripped the fail-closed gate in `emitToolCall` and blocked every tool — including the hidden `yield` — so the revived agent could neither finish nor exit and looped until killed. Both revivers now call the shared `initializeExtensions` helper, restoring runtime actions, `onError`, and the `session_start` event ([#8824](https://github.com/can1357/oh-my-pi/issues/8824)).
 - Fixed `omp commit` split-commit crashing with a misleading `No diff found for <path>` when a staged binary (or any payload) pushed `git diff --cached --binary` past the 8 MiB subprocess output cap. The capture is truncated silently, so files sorting after the binary vanished from the parsed diff; the split flow now requests a complete diff and fails fast naming the real cause instead ([#8897](https://github.com/can1357/oh-my-pi/issues/8897)).

packages/coding-agent/src/commit/git/diff.ts

@@ -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] ?? "";

packages/coding-agent/src/utils/git.ts

@@ -2043,12 +2043,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("");
 	},
 };
 

packages/coding-agent/test/join-patch.test.ts

@@ -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);
+	});
+});