fix(coding-agent): stopped isolated task merges failing when working tree carries WIP for files the agent also modifies

captureRepoDeltaPatch records the delta against `HEAD + WIP`, so the
patch's context lines and blob SHAs reference the WIP-modified files.
commitPatchToBranchWorktree then tried to apply that patch to a fresh
worktree pinned at HEAD, which failed hard whenever the WIP-side file
was missing from HEAD's index (untracked WIP files, staged-new WIP
files) or when --3way could not resolve an overlap.

commitPatchToBranchWorktree now tries plain apply first, then `--3way`
(which cleanly subtracts WIP via the shared ODB blob for tracked files),
and only when both fail replays baseline WIP into the temp worktree so
the delta's context lines up, rewinding WIP-only files afterward so
they never leak into the branch commit.

Added git.ls.tree helper for the WIP-only-file filter and a set of
regression tests covering the untracked, staged-new, and overlap
scenarios.

Fixes #4136
This commit is contained in:
roboomp
2026-07-01 12:24:32 +00:00
parent f70e4f1570
commit 286e971bfe
4 changed files with 263 additions and 8 deletions
+4
View File
@@ -2,6 +2,10 @@
## [Unreleased]
### Fixed
- Fixed isolated task merges failing when the parent working tree carried WIP for a file the isolated subagent also touched. `commitPatchToBranchWorktree` now tries plain apply and `git apply --3way` first (agent-only outcome when the WIP-side blob is tracked in HEAD), then falls back to seeding the temp worktree with the baseline WIP so the delta patch's HEAD+WIP context matches, and rewinds WIP-only files afterward so they don't leak into the branch commit. Covers untracked WIP files, staged-new WIP files, and overlaps `--3way` cannot resolve. ([#4136](https://github.com/can1357/oh-my-pi/issues/4136))
## [16.2.12] - 2026-07-01
### Breaking Changes
+123 -8
View File
@@ -484,41 +484,148 @@ function baselineHasRootWip(baseline: RepoBaseline): boolean {
return !!(baseline.staged.trim() || baseline.unstaged.trim() || baseline.untrackedPatch.trim());
}
/**
* Baseline WIP context needed to safely apply a delta patch whose hunks were
* captured against `HEAD + WIP` (see {@link captureRepoDeltaPatch}). Passed
* whenever {@link baselineHasRootWip} is true so
* {@link commitPatchToBranchWorktree} can replay the WIP into the temp
* worktree first, then rewind WIP-only files after applying the delta.
*/
interface BaselineWipContext {
readonly staged: string;
readonly unstaged: string;
readonly untrackedPatch: string;
/** Untracked file paths present in the baseline (never in HEAD). */
readonly untracked: readonly string[];
}
function collectWipPatches(wip: BaselineWipContext | undefined): string[] {
if (!wip) return [];
return [wip.staged, wip.unstaged, wip.untrackedPatch].filter(p => p.trim());
}
async function commitPatchToBranchWorktree(
tmpDir: string,
taskId: string,
patchText: string,
message: string,
author?: git.CommitAuthor,
baselineWip?: BaselineWipContext,
): Promise<void> {
// Try the two clean paths first — they yield an agent-only commit and are
// the happy case when the temp worktree can resolve the patch against
// HEAD directly:
//
// 1. Plain apply — works when WIP context happens to match HEAD (e.g.
// WIP-touched files that the delta patch doesn't reference).
// 2. `--3way` — works when the WIP-side blob is tracked in HEAD and
// lives in the shared ODB (captureDeltaPatch seeded it while writing
// the synthetic baseline tree). The 3-way merge subtracts WIP,
// producing an agent-only commit even when WIP and agent modify the
// same tracked file at unrelated lines.
//
// If both fail (untracked WIP files, staged-new WIP files, or overlap that
// --3way can't resolve — see #4136), replay the WIP into the worktree
// first so the delta's context lines match, then rewind WIP-only files so
// they don't leak into the commit. Files touched by BOTH WIP and delta
// keep their combined state; the parent's stash-pop reconciles the WIP
// side via 3-way merge on merge-back.
let plainErr: git.GitCommandError | undefined;
try {
await git.patch.applyText(tmpDir, patchText);
} catch (err) {
if (!(err instanceof git.GitCommandError)) throw err;
// Plain apply rejects when the parent checkout carries unrelated dirty
// context near the task's edits; retry with --3way before giving up.
plainErr = err;
}
if (plainErr) {
let threeWayErr: git.GitCommandError | undefined;
try {
await git.patch.applyText(tmpDir, patchText, { threeWay: true });
} catch (threeWayErr) {
if (threeWayErr instanceof git.GitCommandError) {
} catch (err) {
if (!(err instanceof git.GitCommandError)) throw err;
threeWayErr = err;
}
if (threeWayErr) {
const wipPatches = collectWipPatches(baselineWip);
if (wipPatches.length === 0 || !baselineWip) {
const stderr = threeWayErr.result.stderr.slice(0, 2000);
logger.error("commitToBranch: git apply --3way failed", {
taskId,
exitCode: threeWayErr.result.exitCode,
stderr,
initialStderr: err.result.stderr.slice(0, 2000),
initialStderr: plainErr.result.stderr.slice(0, 2000),
patchSize: patchText.length,
patchHead: patchText.slice(0, 500),
});
throw new Error(`git apply --3way failed for task ${taskId}: ${stderr}`);
}
throw threeWayErr;
try {
// `git apply --3way` leaves conflict markers in `U` files when
// it can't resolve; reset the worktree so the WIP-seeded retry
// starts from a clean HEAD tree.
await git.reset(tmpDir, { hard: true, target: "HEAD" });
await applyDeltaOverBaselineWip(tmpDir, taskId, patchText, wipPatches, baselineWip);
} catch (wipErr) {
if (!(wipErr instanceof git.GitCommandError)) throw wipErr;
const stderr = wipErr.result.stderr.slice(0, 2000);
logger.error("commitToBranch: git apply with baseline WIP failed", {
taskId,
exitCode: wipErr.result.exitCode,
stderr,
threeWayStderr: threeWayErr.result.stderr.slice(0, 2000),
initialStderr: plainErr.result.stderr.slice(0, 2000),
patchSize: patchText.length,
patchHead: patchText.slice(0, 500),
});
throw new Error(`git apply with baseline WIP failed for task ${taskId}: ${stderr}`);
}
}
}
await git.stage.files(tmpDir);
await git.commit(tmpDir, message, author ? { author } : {});
}
/**
* Replay baseline WIP into the temp worktree so the delta patch's HEAD+WIP
* context matches, apply the delta, then rewind files WIP touched but the
* delta didn't — HEAD-tracked files are restored via `git restore`, untracked
* or staged-new WIP files are removed from the worktree. The commit that
* follows reflects agent's delta plus any overlap with WIP; parent's
* stash-pop reconciles the WIP side on merge-back.
*/
async function applyDeltaOverBaselineWip(
tmpDir: string,
_taskId: string,
patchText: string,
wipPatches: readonly string[],
baselineWip: BaselineWipContext,
): Promise<void> {
for (const wip of wipPatches) {
await git.patch.applyText(tmpDir, wip);
}
await git.patch.applyText(tmpDir, patchText);
const wipFiles = new Set(wipPatches.flatMap(patchTouchedFiles));
const deltaFiles = new Set(patchTouchedFiles(patchText));
const wipOnly = [...wipFiles].filter(f => !deltaFiles.has(f));
if (wipOnly.length === 0) return;
// Any wipOnly file baselined as untracked cannot be in HEAD.
// Everything else may or may not — verify against HEAD's tree.
const untrackedSet = new Set(baselineWip.untracked);
const candidates = wipOnly.filter(f => !untrackedSet.has(f));
const inHead = candidates.length > 0 ? new Set(await git.ls.tree(tmpDir, "HEAD", candidates)) : new Set<string>();
const toRestore = candidates.filter(f => inHead.has(f));
const toRemove = wipOnly.filter(f => !toRestore.includes(f));
if (toRestore.length > 0) {
await git.restore(tmpDir, { source: "HEAD", staged: true, worktree: true, files: toRestore });
}
for (const rel of toRemove) {
await fs.rm(path.join(tmpDir, rel), { force: true });
}
}
interface FilteredAgentReplayOptions {
baseline: WorktreeBaseline;
branchName: string;
@@ -576,7 +683,14 @@ async function replayFilteredAgentCommits(opts: FilteredAgentReplayOptions): Pro
});
if (leftoverPatch.trim()) {
const msg = (opts.commitMessage && (await opts.commitMessage(leftoverPatch))) || opts.fallbackMessage;
await commitPatchToBranchWorktree(tmpDir, opts.taskId, leftoverPatch, msg);
// Only seed baseline WIP for the leftover patch when the loop body never
// ran — the tmpDir worktree is still pinned at baselineSha and the
// leftover collapses to the raw rootPatch (whose context is WIP-based).
// After any filtered agent commit landed, tmpDir has advanced past
// baselineSha and leftoverPatch is HEAD-derived; applying WIP would
// corrupt it.
const wip = agentCommits.length === 0 ? opts.baseline.root : undefined;
await commitPatchToBranchWorktree(tmpDir, opts.taskId, leftoverPatch, msg, undefined, wip);
}
} finally {
await git.worktree.tryRemove(opts.repoRoot, tmpDir);
@@ -677,7 +791,8 @@ export async function commitToBranch(
await git.worktree.add(repoRoot, tmpDir, branchName);
const msg = (commitMessage && (await commitMessage(rootPatch))) || fallbackMessage;
await commitPatchToBranchWorktree(tmpDir, taskId, rootPatch, msg);
const wip = baselineHasRootWip(baseline.root) ? baseline.root : undefined;
await commitPatchToBranchWorktree(tmpDir, taskId, rootPatch, msg, undefined, wip);
} finally {
await git.worktree.tryRemove(repoRoot, tmpDir);
await fs.rm(tmpDir, { recursive: true, force: true });
+8
View File
@@ -1674,6 +1674,14 @@ export const ls = {
return ls.files(cwd, { others: true, excludeStandard: true, signal });
},
/** List paths present in a ref, optionally filtered to specific paths. */
async tree(cwd: string, ref: string, files: readonly string[] = [], signal?: AbortSignal): Promise<string[]> {
const args = ["ls-tree", "--name-only", "-r", "-z", ref];
if (files.length > 0) args.push("--", ...files);
const raw = await runText(cwd, args, { readOnly: true, signal });
return raw.split("\0").filter(entry => entry.length > 0);
},
/** List submodule paths (recursive). */
async submodules(cwd: string, signal?: AbortSignal): Promise<string[]> {
const output = await git(cwd, ["submodule", "--quiet", "foreach", "--recursive", "echo $sm_path"], {
@@ -641,4 +641,132 @@ describe("commitToBranch preserves agent commits", () => {
const result = await commitToBranch(isolation, baseline, "empty", undefined);
expect(result).toBeNull();
});
// Regression: #4136. Baseline WIP made captureRepoDeltaPatch record hunks
// against `HEAD + WIP`; commitPatchToBranchWorktree then failed to apply
// those hunks to a fresh worktree pinned at HEAD when the WIP-side file was
// missing from HEAD's index (untracked WIP files, staged-new WIP files) or
// when --3way couldn't resolve the overlap. Each scenario below reproduced
// the failure before the fix.
describe("with baseline WIP overlapping the agent's changes (#4136)", () => {
async function seedWipFileFromParent(destRoot: string, relPath: string): Promise<void> {
await fs.mkdir(path.join(destRoot, path.dirname(relPath)), { recursive: true });
await fs.copyFile(path.join(parent, relPath), path.join(destRoot, relPath));
}
it("commits an agent-only delta via --3way when WIP and agent modify unrelated hunks of the same tracked file", async () => {
const fixture = "src/foo.py";
const head = Array.from({ length: 40 }, (_, i) => `# line ${i + 1}\n`).join("");
await fs.mkdir(path.join(parent, "src"), { recursive: true });
await fs.writeFile(path.join(parent, fixture), head);
await gitr(parent, ["add", "."]);
await gitr(parent, ["commit", "-q", "-m", "add fixture"]);
// Isolation must be re-cloned so the fixture is present in HEAD.
await fs.rm(isolation, { recursive: true, force: true });
await gitr(parent, ["clone", "-q", "--no-hardlinks", "--local", parent, isolation]);
await gitr(isolation, ["config", "user.email", "agent@example.com"]);
await gitr(isolation, ["config", "user.name", "Agent User"]);
// Parent WIP: change line 10 (unstaged edit to an existing tracked file).
const wipLines = head.split("\n");
wipLines[9] = "# line 10 thinkingLevel: medium";
await fs.writeFile(path.join(parent, fixture), wipLines.join("\n"));
await seedWipFileFromParent(isolation, fixture);
// Agent modifies line 30, far from the WIP change.
const agentLines = wipLines.slice();
agentLines[29] = "# line 30 def new_func()";
await fs.writeFile(path.join(isolation, fixture), agentLines.join("\n"));
const baseline = await captureBaseline(parent);
const result = await commitToBranch(isolation, baseline, "wip-tracked-file", undefined);
expect(result?.branchName).toBe("omp/task/wip-tracked-file");
const branchDiff = await gitr(parent, ["show", "--pretty=format:", result!.branchName!]);
expect(branchDiff).toContain("+# line 30 def new_func()");
// --3way must subtract the WIP change from the commit; only the
// agent's line 30 edit belongs on the task branch.
expect(branchDiff).not.toContain("thinkingLevel: medium");
});
it("commits an untracked WIP file that the agent modifies inside isolation", async () => {
// Parent WIP: add a new untracked file the agent also touches.
await fs.mkdir(path.join(parent, "src"), { recursive: true });
await fs.writeFile(path.join(parent, "src/new.py"), "WIP header\nunchanged\n");
await fs.mkdir(path.join(isolation, "src"), { recursive: true });
await fs.copyFile(path.join(parent, "src/new.py"), path.join(isolation, "src/new.py"));
// Agent extends the file inside isolation.
await fs.writeFile(path.join(isolation, "src/new.py"), "WIP header\nagent-edit\n");
const baseline = await captureBaseline(parent);
expect(baseline.root.untracked).toContain("src/new.py");
const result = await commitToBranch(isolation, baseline, "wip-untracked", undefined);
expect(result?.branchName).toBe("omp/task/wip-untracked");
const branchDiff = await gitr(parent, ["show", "--pretty=format:", result!.branchName!]);
expect(branchDiff).toContain("new file mode");
expect(branchDiff).toContain("src/new.py");
expect(branchDiff).toContain("+WIP header");
expect(branchDiff).toContain("+agent-edit");
});
it("commits a staged-new WIP file that the agent modifies inside isolation", async () => {
// Parent WIP: stage a new file that isn't yet in HEAD.
await fs.writeFile(path.join(parent, "notes.md"), "l1\nl2\nl3\n");
await gitr(parent, ["add", "notes.md"]);
await fs.copyFile(path.join(parent, "notes.md"), path.join(isolation, "notes.md"));
await gitr(isolation, ["add", "notes.md"]);
// Agent edits the staged-new file.
await fs.writeFile(path.join(isolation, "notes.md"), "l1\nl2 agent\nl3\n");
const baseline = await captureBaseline(parent);
expect(baseline.root.staged).toContain("new file mode");
const result = await commitToBranch(isolation, baseline, "wip-staged-new", undefined);
expect(result?.branchName).toBe("omp/task/wip-staged-new");
const branchDiff = await gitr(parent, ["show", "--pretty=format:", result!.branchName!]);
expect(branchDiff).toContain("new file mode");
expect(branchDiff).toContain("notes.md");
expect(branchDiff).toContain("+l2 agent");
});
it("does not leak WIP-only files into the branch commit when the agent leaves them untouched", async () => {
// Parent WIP: touch two files. The agent only edits `wanted.py`;
// `wip-only.py` must not appear in the branch commit at all.
await fs.mkdir(path.join(parent, "src"), { recursive: true });
await fs.writeFile(path.join(parent, "src/wanted.py"), "unchanged\n");
await fs.writeFile(path.join(parent, "src/wip-only.py"), "unchanged\n");
await gitr(parent, ["add", "."]);
await gitr(parent, ["commit", "-q", "-m", "seed"]);
await fs.rm(isolation, { recursive: true, force: true });
await gitr(parent, ["clone", "-q", "--no-hardlinks", "--local", parent, isolation]);
await gitr(isolation, ["config", "user.email", "agent@example.com"]);
await gitr(isolation, ["config", "user.name", "Agent User"]);
await fs.writeFile(path.join(parent, "src/wip-only.py"), "wip edit\n");
await fs.writeFile(path.join(parent, "src/wanted.py"), "wip mixed\n");
await fs.copyFile(path.join(parent, "src/wip-only.py"), path.join(isolation, "src/wip-only.py"));
await fs.copyFile(path.join(parent, "src/wanted.py"), path.join(isolation, "src/wanted.py"));
// Untracked WIP file the agent also does not touch.
await fs.writeFile(path.join(parent, "user-wip.txt"), "user wip\n");
await fs.copyFile(path.join(parent, "user-wip.txt"), path.join(isolation, "user-wip.txt"));
// Agent only extends `wanted.py`; leaves `wip-only.py` and
// `user-wip.txt` alone.
await fs.writeFile(path.join(isolation, "src/wanted.py"), "wip mixed\nagent line\n");
const baseline = await captureBaseline(parent);
const result = await commitToBranch(isolation, baseline, "wip-filter", undefined);
expect(result?.branchName).toBe("omp/task/wip-filter");
const files = (await gitr(parent, ["show", "--name-only", "--pretty=format:", result!.branchName!]))
.split("\n")
.filter(Boolean);
// Only the agent-touched file lands on the branch — no WIP-only files.
expect(files).toEqual(["src/wanted.py"]);
});
});
});