fix(coding-agent): preserve agent commits across isolated branch merges

When an isolated task agent commits its own changes before yielding, the
harness used to collapse the captured delta into one AI-summarized commit
and discard the agent's commit messages and authorship entirely. This
violated commit discipline for agentic swarms — multiple logical commits
("fix bug" + "add test") became a single opaque commit, and the
agent's commit object (which lived in isolation/.git/objects under
overlayfs/rcopy) was lost when cleanupIsolation tore down the overlay.

commitToBranch now detects when isolation HEAD moved past baseline.root
.headCommit. When it has, the function git-fetches the agent's HEAD into
the parent repo as omp/task/${taskId} so the commit objects survive
cleanupIsolation, and stamps the captured baselineSha onto the returned
CommitToBranchResult. mergeTaskBranches cherry-picks the inclusive range
baseSha..branchName when baseSha is provided, replaying each agent
commit verbatim with its original message and author. Any uncommitted
leftover (staged, unstaged, untracked) on top of the agent's last commit
becomes one trailing AI-summarized commit on the same branch.

Falls back to the legacy single-commit path when the agent never moved
HEAD (purely dirty working tree); existing patch-mode flow is untouched.

Fixes #3842
This commit is contained in:
roboomp
2026-06-30 00:13:07 +00:00
parent 0ba736f5bc
commit da715aae7c
5 changed files with 228 additions and 17 deletions
+4
View File
@@ -2,6 +2,10 @@
## [Unreleased]
### Fixed
- Isolated branch-mode task merges now preserve the agent's own commits (message and author) instead of collapsing every diff into a single AI-summarized commit. `commitToBranch` detects when the subagent moved HEAD past the baseline, transfers the new commit objects into the parent repo via `git fetch`, and `mergeTaskBranches` cherry-picks the inclusive range `baseSha..omp/task/<id>` so each commit replays verbatim; any uncommitted leftover on top of the agent's last commit lands as one trailing AI-summarized commit ([#3842](https://github.com/can1357/oh-my-pi/issues/3842)).
## [16.2.6] - 2026-06-29
### Changed
@@ -154,6 +154,7 @@ export async function runIsolatedSubprocess(opts: IsolatedRunOptions): Promise<S
return {
...result,
branchName: commitResult?.branchName,
branchBaseSha: commitResult?.baseSha,
nestedPatches: commitResult?.nestedPatches,
};
} catch (mergeErr) {
@@ -235,7 +236,12 @@ export async function mergeIsolatedChanges(opts: IsolationMergeOptions): Promise
};
}
const mergeResult = await mergeTaskBranches(repoRoot, [
{ branchName: result.branchName, taskId: result.id, description: result.description },
{
branchName: result.branchName,
taskId: result.id,
description: result.description,
baseSha: result.branchBaseSha,
},
]);
const mergedBranchForNestedPatches = mergeResult.merged.includes(result.branchName);
const changesApplied = mergeResult.failed.length === 0;
+6
View File
@@ -383,6 +383,12 @@ export interface SingleResult {
patchPath?: string;
/** Branch name for isolated branch-mode output */
branchName?: string;
/**
* Baseline commit SHA the task branch was created from. Passed to
* `mergeTaskBranches` so cherry-pick uses the inclusive range
* `branchBaseSha..branchName` and preserves every agent commit's message.
*/
branchBaseSha?: string;
/** Nested repo patches to apply after parent merge */
nestedPatches?: NestedRepoPatch[];
/** Data extracted by registered subprocess tool handlers (keyed by tool name) */
+76 -16
View File
@@ -460,12 +460,33 @@ export async function cleanupIsolation(handle: IsolationHandle): Promise<void> {
export interface CommitToBranchResult {
branchName?: string;
nestedPatches: NestedRepoPatch[];
/**
* SHA of the parent-repo commit the task branch was created on top of, so
* {@link mergeTaskBranches} can cherry-pick the range `baseSha..branchName`
* and preserve every agent commit's message and author.
*/
baseSha?: string;
}
/**
* Commit task-only changes to a new branch.
* Only root repo changes go on the branch. Nested repo patches are returned
* separately since the parent git can't track files inside gitlinks.
* Capture task-only changes from the isolation worktree onto a parent-repo
* branch named `omp/task/${taskId}`. Only root-repo changes go on the branch;
* nested-repo patches are returned separately because the parent git can't
* track files inside gitlinks.
*
* If the agent committed its own changes inside isolation (HEAD moved past
* `baseline.root.headCommit`), this transfers those commit objects into the
* parent repo via `git fetch` and points the branch at the agent's HEAD —
* later cherry-pick of `baseSha..branchName` replays every commit with its
* original message and author preserved. Any uncommitted leftover (staged,
* unstaged, untracked) on top of the agent's last commit becomes one
* additional commit with an AI-generated message.
*
* If the agent did not commit, the captured delta is collapsed onto a single
* branch commit with an AI-generated (or fallback) message — the legacy
* behaviour.
*
* Returns `null` when no root or nested changes exist.
*/
export async function commitToBranch(
isolationDir: string,
@@ -474,6 +495,10 @@ export async function commitToBranch(
description: string | undefined,
commitMessage?: (diff: string) => Promise<string | null>,
): Promise<CommitToBranchResult | null> {
const baselineSha = baseline.root.headCommit;
const isolationHead = (await git.head.sha(isolationDir)) ?? "";
const agentCommitted = isolationHead !== "" && isolationHead !== baselineSha;
const { rootPatch, nestedPatches } = await captureDeltaPatch(isolationDir, baseline);
if (!rootPatch.trim() && nestedPatches.length === 0) return null;
@@ -481,14 +506,40 @@ export async function commitToBranch(
const branchName = `omp/task/${taskId}`;
const fallbackMessage = description || taskId;
// Only create a branch if the root repo has changes
if (rootPatch.trim()) {
let branchCreated = false;
let leftoverPatch = "";
if (agentCommitted) {
// Transfer the agent's commit objects (which live in isolation's `.git`,
// stranded once `cleanupIsolation` tears the overlay down) into the parent
// repo's object DB and create the branch at the agent's HEAD. `+HEAD:…`
// force-overwrites a stale branch from a prior run.
await git.fetch(repoRoot, isolationDir, "HEAD", `refs/heads/${branchName}`);
branchCreated = true;
// Leftover = anything still uncommitted in isolation on top of the
// agent's last commit (staged, unstaged, untracked). The agent didn't
// commit it, so it goes in as one AI-summarized trailing commit.
leftoverPatch = await captureRepoDeltaPatch(isolationDir, {
repoRoot: isolationDir,
headCommit: isolationHead,
staged: "",
unstaged: "",
untracked: [],
untrackedPatch: "",
});
} else if (rootPatch.trim()) {
await git.branch.create(repoRoot, branchName);
branchCreated = true;
leftoverPatch = rootPatch;
}
if (branchCreated && leftoverPatch.trim()) {
const tmpDir = path.join(os.tmpdir(), `omp-branch-${Snowflake.next()}`);
try {
await git.worktree.add(repoRoot, tmpDir, branchName);
try {
await git.patch.applyText(tmpDir, rootPatch);
await git.patch.applyText(tmpDir, leftoverPatch);
} catch (err) {
if (err instanceof git.GitCommandError) {
const stderr = err.result.stderr.slice(0, 2000);
@@ -496,15 +547,15 @@ export async function commitToBranch(
taskId,
exitCode: err.result.exitCode,
stderr,
patchSize: rootPatch.length,
patchHead: rootPatch.slice(0, 500),
patchSize: leftoverPatch.length,
patchHead: leftoverPatch.slice(0, 500),
});
throw new Error(`git apply failed for task ${taskId}: ${stderr}`);
}
throw err;
}
await git.stage.files(tmpDir);
const msg = (commitMessage && (await commitMessage(rootPatch))) || fallbackMessage;
const msg = (commitMessage && (await commitMessage(leftoverPatch))) || fallbackMessage;
await git.commit(tmpDir, msg);
} finally {
await git.worktree.tryRemove(repoRoot, tmpDir);
@@ -512,7 +563,11 @@ export async function commitToBranch(
}
}
return { branchName: rootPatch.trim() ? branchName : undefined, nestedPatches };
return {
branchName: branchCreated ? branchName : undefined,
baseSha: baselineSha,
nestedPatches,
};
}
export interface MergeBranchResult {
@@ -524,13 +579,17 @@ export interface MergeBranchResult {
}
/**
* Cherry-pick task branch commits sequentially onto HEAD.
* Each branch has a single commit that gets replayed cleanly.
* Stops on first conflict and reports which branches succeeded.
* Cherry-pick task branch commits sequentially onto HEAD. When `baseSha` is
* provided the cherry-pick uses the inclusive range `baseSha..branchName`,
* replaying every commit individually and preserving each commit's message
* and author. When omitted, the branch is cherry-picked as a single commit
* (legacy callers).
*
* Stops on the first conflict and reports which branches succeeded.
*/
export async function mergeTaskBranches(
repoRoot: string,
branches: Array<{ branchName: string; taskId: string; description?: string }>,
branches: Array<{ branchName: string; taskId: string; description?: string; baseSha?: string }>,
): Promise<MergeBranchResult> {
// Serialize against other in-process git mutations on this repo: concurrent
// background merges interleaving stash push/pop + cherry-pick would corrupt
@@ -546,9 +605,10 @@ export async function mergeTaskBranches(
let conflictResult: MergeBranchResult | undefined;
try {
for (const { branchName } of branches) {
for (const { branchName, baseSha } of branches) {
try {
await git.cherryPick(repoRoot, branchName);
const target = baseSha ? `${baseSha}..${branchName}` : branchName;
await git.cherryPick(repoRoot, target);
} catch (err) {
try {
await git.cherryPick.abort(repoRoot);
@@ -6,6 +6,7 @@ import {
applyNestedPatches,
captureBaseline,
captureDeltaPatch,
commitToBranch,
ensureIsolation,
getGitNoIndexNullPath,
getRepoRoot,
@@ -426,3 +427,137 @@ describe("applyNestedPatches", () => {
expect(stashList).toContain("omp-isolation-");
});
});
describe("commitToBranch preserves agent commits", () => {
let parent: string;
let isolation: string;
async function gitr(repo: string, args: string[]): Promise<string> {
return runGit(repo, args);
}
beforeEach(async () => {
parent = await fs.mkdtemp(path.join(os.tmpdir(), "omp-commit-parent-"));
isolation = await fs.mkdtemp(path.join(os.tmpdir(), "omp-commit-iso-"));
await gitr(parent, ["init", "-q", "-b", "main"]);
await gitr(parent, ["config", "user.email", "user@example.com"]);
await gitr(parent, ["config", "user.name", "Parent User"]);
await fs.writeFile(
path.join(parent, "EXP_CLEAN_COMMIT.txt"),
"line1\nline2\nline3\nline4\nline5\nline6\nline7\nline8\nline9\nline10\n",
);
await gitr(parent, ["add", "."]);
await gitr(parent, ["commit", "-q", "-m", "add clean test fixture"]);
// Simulate copy-on-write isolation: a real local clone so the agent's
// commit objects live in `isolation/.git`, just like the overlay/rcopy
// isolation backends would arrange them at runtime.
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"]);
});
afterEach(async () => {
await Promise.all([removeWithRetries(parent), removeWithRetries(isolation)]);
});
// Reproduces issue #3842: agent commits with a specific message inside
// isolation; the merged commit on the parent branch must keep that exact
// message instead of an AI-generated summary.
it("preserves the agent's commit message after merge", async () => {
const baseline = await captureBaseline(parent);
await fs.writeFile(
path.join(isolation, "EXP_CLEAN_COMMIT.txt"),
"line1\nline2\nline3\nline4\nLINE5-AGENT-WITH-MESSAGE\nline6\nline7\nline8\nline9\nline10\n",
);
await gitr(isolation, ["add", "EXP_CLEAN_COMMIT.txt"]);
const agentMessage = "fix(test): agent committed with specific message for preservation check";
await gitr(isolation, ["commit", "-q", "-m", agentMessage]);
const taskId = "preservation-check";
const aiMessage = vi.fn(async () => "fix: update line5 in clean commit example");
const result = await commitToBranch(isolation, baseline, taskId, undefined, aiMessage);
expect(result?.branchName).toBe(`omp/task/${taskId}`);
expect(result?.baseSha).toBe(baseline.root.headCommit);
// commitMessage callback must NOT have been invoked — the agent's
// message is taken verbatim.
expect(aiMessage).not.toHaveBeenCalled();
const branchSubject = await gitr(parent, ["log", "-1", "--pretty=%s", result!.branchName!]);
expect(branchSubject).toBe(agentMessage);
const merge = await mergeTaskBranches(parent, [
{ branchName: result!.branchName!, taskId, baseSha: result!.baseSha! },
]);
expect(merge.failed).toEqual([]);
expect(merge.merged).toEqual([result!.branchName!]);
const headSubject = await gitr(parent, ["log", "-1", "--pretty=%s"]);
expect(headSubject).toBe(agentMessage);
});
it("preserves every message when the agent makes multiple commits", async () => {
const baseline = await captureBaseline(parent);
await fs.writeFile(path.join(isolation, "a.txt"), "alpha\n");
await gitr(isolation, ["add", "a.txt"]);
await gitr(isolation, ["commit", "-q", "-m", "feat: add alpha file"]);
await fs.writeFile(path.join(isolation, "b.txt"), "beta\n");
await gitr(isolation, ["add", "b.txt"]);
await gitr(isolation, ["commit", "-q", "-m", "test: add beta coverage"]);
const result = await commitToBranch(isolation, baseline, "multi", undefined);
expect(result?.branchName).toBe("omp/task/multi");
const merge = await mergeTaskBranches(parent, [
{ branchName: result!.branchName!, taskId: "multi", baseSha: result!.baseSha! },
]);
expect(merge).toEqual({ failed: [], merged: ["omp/task/multi"] });
const subjects = (await gitr(parent, ["log", "-2", "--pretty=%s"])).split("\n");
expect(subjects).toEqual(["test: add beta coverage", "feat: add alpha file"]);
});
it("appends one trailing commit when the agent leaves uncommitted work after committing", async () => {
const baseline = await captureBaseline(parent);
await fs.writeFile(path.join(isolation, "a.txt"), "alpha\n");
await gitr(isolation, ["add", "a.txt"]);
await gitr(isolation, ["commit", "-q", "-m", "feat: add alpha file"]);
// Uncommitted change on top of the agent's commit — should land as one
// extra commit with the AI-generated message, NOT silently dropped.
await fs.writeFile(path.join(isolation, "b.txt"), "beta\n");
const aiMessage = vi.fn(async () => "chore: leftover beta wip");
const result = await commitToBranch(isolation, baseline, "leftover", undefined, aiMessage);
expect(result?.branchName).toBe("omp/task/leftover");
expect(aiMessage).toHaveBeenCalledTimes(1);
const subjects = (await gitr(parent, ["log", "-2", "--pretty=%s", result!.branchName!])).split("\n");
expect(subjects).toEqual(["chore: leftover beta wip", "feat: add alpha file"]);
});
it("falls back to the AI-generated message when the agent never committed", async () => {
const baseline = await captureBaseline(parent);
await fs.writeFile(path.join(isolation, "a.txt"), "alpha\n");
const aiMessage = vi.fn(async () => "feat: add alpha");
const result = await commitToBranch(isolation, baseline, "nocommit", undefined, aiMessage);
expect(result?.branchName).toBe("omp/task/nocommit");
expect(aiMessage).toHaveBeenCalledTimes(1);
const branchSubject = await gitr(parent, ["log", "-1", "--pretty=%s", result!.branchName!]);
expect(branchSubject).toBe("feat: add alpha");
});
it("returns null when nothing changed in isolation", async () => {
const baseline = await captureBaseline(parent);
const result = await commitToBranch(isolation, baseline, "empty", undefined);
expect(result).toBeNull();
});
});