From 66052863bb6b7e8b95404426d8f94de79522153e Mon Sep 17 00:00:00 2001 From: roboomp Date: Wed, 1 Jul 2026 11:58:52 +0000 Subject: [PATCH] fix(agent): handled already-applied patch merges Use git apply --3way for patch-mode isolated merge checks and applies so diff-tree patches that are already present are accepted as no-ops. Added regression coverage for a clean already-applied binary/full-index patch. Fixes #4135 --- .../coding-agent/src/task/isolation-runner.ts | 4 +- .../test/task/isolation-runner.test.ts | 50 ++++++++++++++++++- 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/packages/coding-agent/src/task/isolation-runner.ts b/packages/coding-agent/src/task/isolation-runner.ts index 5231b90b7..0fba6799c 100644 --- a/packages/coding-agent/src/task/isolation-runner.ts +++ b/packages/coding-agent/src/task/isolation-runner.ts @@ -291,11 +291,11 @@ export async function mergeIsolatedChanges(opts: IsolationMergeOptions): Promise hadAnyChanges = false; } else { const normalized = patchText.endsWith("\n") ? patchText : `${patchText}\n`; - changesApplied = await git.patch.canApplyText(repoRoot, normalized); + changesApplied = await git.patch.canApplyText(repoRoot, normalized, { threeWay: true }); hadAnyChanges = false; if (changesApplied) { try { - await git.patch.applyText(repoRoot, normalized); + await git.patch.applyText(repoRoot, normalized, { threeWay: true }); hadAnyChanges = true; } catch { changesApplied = false; diff --git a/packages/coding-agent/test/task/isolation-runner.test.ts b/packages/coding-agent/test/task/isolation-runner.test.ts index e8142c3f7..38dcd65b0 100644 --- a/packages/coding-agent/test/task/isolation-runner.test.ts +++ b/packages/coding-agent/test/task/isolation-runner.test.ts @@ -1,3 +1,7 @@ +import { $ } from "bun"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; import { afterEach, describe, expect, it, vi } from "bun:test"; import { applyEligibleNestedPatches, mergeIsolatedChanges } from "@oh-my-pi/pi-coding-agent/task/isolation-runner"; import type { SingleResult } from "@oh-my-pi/pi-coding-agent/task/types"; @@ -22,9 +26,39 @@ function result(overrides: Partial = {}): SingleResult { }; } +const tempRoots: string[] = []; + +async function git(repoRoot: string, ...args: string[]): Promise { + const result = await $`git ${args}`.cwd(repoRoot).quiet().nothrow(); + if (result.exitCode !== 0) { + throw new Error(`git ${args.join(" ")} failed: ${result.stderr.toString()}`); + } + return result.text(); +} + +async function makeAlreadyAppliedPatchRepo(): Promise<{ repoRoot: string; patchPath: string }> { + const repoRoot = await fs.mkdtemp(path.join(os.tmpdir(), "omp-isolation-merge-")); + tempRoots.push(repoRoot); + + await git(repoRoot, "init"); + await git(repoRoot, "config", "user.email", "repro@example.com"); + await git(repoRoot, "config", "user.name", "Repro"); + await Bun.write(path.join(repoRoot, "foo.txt"), "old\n"); + await git(repoRoot, "add", "foo.txt"); + await git(repoRoot, "commit", "-m", "base"); + await Bun.write(path.join(repoRoot, "foo.txt"), "new\n"); + await git(repoRoot, "commit", "-am", "change"); + + const patchPath = path.join(repoRoot, "task.patch"); + const patchText = await git(repoRoot, "diff-tree", "--binary", "--full-index", "--no-commit-id", "-p", "HEAD"); + await Bun.write(patchPath, patchText); + return { repoRoot, patchPath }; +} + describe("mergeIsolatedChanges", () => { - afterEach(() => { + afterEach(async () => { vi.restoreAllMocks(); + await Promise.all(tempRoots.splice(0).map(tempRoot => fs.rm(tempRoot, { force: true, recursive: true }))); }); it("allows nested-only branch-mode patches to apply when no root branch was created", async () => { @@ -63,6 +97,20 @@ describe("mergeIsolatedChanges", () => { expect(outcome.summary).not.toContain("No changes to apply"); }); + it("treats already-applied patch-mode diffs as successful no-ops", async () => { + const { repoRoot, patchPath } = await makeAlreadyAppliedPatchRepo(); + + const outcome = await mergeIsolatedChanges({ + repoRoot, + mergeMode: "patch", + result: result({ patchPath }), + }); + + expect(outcome.changesApplied).toBe(true); + expect(outcome.summary).not.toContain("Patches were not applied"); + expect(await git(repoRoot, "status", "--porcelain", "--", "foo.txt")).toBe(""); + }); + it("does not mark failed branch-mode runs as nested-patch eligible", async () => { const outcome = await mergeIsolatedChanges({ repoRoot: "/repo",