feat(coding-agent): preserve results on isolated merge failures
This commit is contained in:
DeprecatedLuke
@@ -978,82 +978,90 @@ export class TaskTool implements AgentTool<TaskSchema, TaskToolDetails, Theme> {
|
||||
let changesApplied: boolean | null = null;
|
||||
let mergedBranchesForNestedPatches: Set<string> | null = null;
|
||||
if (isIsolated && repoRoot) {
|
||||
if (mergeMode === "branch") {
|
||||
// Branch mode: merge task branches sequentially
|
||||
const branchEntries = results
|
||||
.filter(r => r.branchName && r.exitCode === 0 && !r.aborted)
|
||||
.map(r => ({ branchName: r.branchName!, taskId: r.id, description: r.description }));
|
||||
try {
|
||||
if (mergeMode === "branch") {
|
||||
// Branch mode: merge task branches sequentially
|
||||
const branchEntries = results
|
||||
.filter(r => r.branchName && r.exitCode === 0 && !r.aborted)
|
||||
.map(r => ({ branchName: r.branchName!, taskId: r.id, description: r.description }));
|
||||
|
||||
if (branchEntries.length === 0) {
|
||||
changesApplied = true;
|
||||
} else {
|
||||
const mergeResult = await mergeTaskBranches(repoRoot, branchEntries);
|
||||
mergedBranchesForNestedPatches = new Set(mergeResult.merged);
|
||||
changesApplied = mergeResult.failed.length === 0;
|
||||
|
||||
if (changesApplied) {
|
||||
mergeSummary = `\n\nMerged ${mergeResult.merged.length} branch${mergeResult.merged.length === 1 ? "" : "es"}: ${mergeResult.merged.join(", ")}`;
|
||||
} else {
|
||||
const mergedPart =
|
||||
mergeResult.merged.length > 0 ? `Merged: ${mergeResult.merged.join(", ")}.\n` : "";
|
||||
const failedPart = `Failed: ${mergeResult.failed.join(", ")}.`;
|
||||
const conflictPart = mergeResult.conflict ? `\nConflict: ${mergeResult.conflict}` : "";
|
||||
mergeSummary = `\n\n<system-notification>Branch merge failed. ${mergedPart}${failedPart}${conflictPart}\nUnmerged branches remain for manual resolution.</system-notification>`;
|
||||
}
|
||||
}
|
||||
|
||||
// Clean up merged branches (keep failed ones for manual resolution)
|
||||
const allBranches = branchEntries.map(b => b.branchName);
|
||||
if (changesApplied) {
|
||||
await cleanupTaskBranches(repoRoot, allBranches);
|
||||
}
|
||||
} else {
|
||||
// Patch mode: combine and apply patches
|
||||
const patchesInOrder = results.map(result => result.patchPath).filter(Boolean) as string[];
|
||||
const missingPatch = results.some(result => !result.patchPath);
|
||||
if (missingPatch) {
|
||||
changesApplied = false;
|
||||
} else {
|
||||
const patchStats = await Promise.all(
|
||||
patchesInOrder.map(async patchPath => ({
|
||||
patchPath,
|
||||
size: (await fs.stat(patchPath)).size,
|
||||
})),
|
||||
);
|
||||
const nonEmptyPatches = patchStats.filter(patch => patch.size > 0).map(patch => patch.patchPath);
|
||||
if (nonEmptyPatches.length === 0) {
|
||||
if (branchEntries.length === 0) {
|
||||
changesApplied = true;
|
||||
} else {
|
||||
const patchTexts = await Promise.all(
|
||||
nonEmptyPatches.map(async patchPath => Bun.file(patchPath).text()),
|
||||
const mergeResult = await mergeTaskBranches(repoRoot, branchEntries);
|
||||
mergedBranchesForNestedPatches = new Set(mergeResult.merged);
|
||||
changesApplied = mergeResult.failed.length === 0;
|
||||
|
||||
if (changesApplied) {
|
||||
mergeSummary = `\n\nMerged ${mergeResult.merged.length} branch${mergeResult.merged.length === 1 ? "" : "es"}: ${mergeResult.merged.join(", ")}`;
|
||||
} else {
|
||||
const mergedPart =
|
||||
mergeResult.merged.length > 0 ? `Merged: ${mergeResult.merged.join(", ")}.\n` : "";
|
||||
const failedPart = `Failed: ${mergeResult.failed.join(", ")}.`;
|
||||
const conflictPart = mergeResult.conflict ? `\nConflict: ${mergeResult.conflict}` : "";
|
||||
mergeSummary = `\n\n<system-notification>Branch merge failed. ${mergedPart}${failedPart}${conflictPart}\nUnmerged branches remain for manual resolution.</system-notification>`;
|
||||
}
|
||||
}
|
||||
|
||||
// Clean up merged branches (keep failed ones for manual resolution)
|
||||
const allBranches = branchEntries.map(b => b.branchName);
|
||||
if (changesApplied) {
|
||||
await cleanupTaskBranches(repoRoot, allBranches);
|
||||
}
|
||||
} else {
|
||||
// Patch mode: combine and apply patches
|
||||
const patchesInOrder = results.map(result => result.patchPath).filter(Boolean) as string[];
|
||||
const missingPatch = results.some(result => !result.patchPath);
|
||||
if (missingPatch) {
|
||||
changesApplied = false;
|
||||
} else {
|
||||
const patchStats = await Promise.all(
|
||||
patchesInOrder.map(async patchPath => ({
|
||||
patchPath,
|
||||
size: (await fs.stat(patchPath)).size,
|
||||
})),
|
||||
);
|
||||
const combinedPatch = patchTexts.map(text => (text.endsWith("\n") ? text : `${text}\n`)).join("");
|
||||
if (!combinedPatch.trim()) {
|
||||
const nonEmptyPatches = patchStats.filter(patch => patch.size > 0).map(patch => patch.patchPath);
|
||||
if (nonEmptyPatches.length === 0) {
|
||||
changesApplied = true;
|
||||
} else {
|
||||
changesApplied = await git.patch.canApplyText(repoRoot, combinedPatch);
|
||||
if (changesApplied) {
|
||||
try {
|
||||
await git.patch.applyText(repoRoot, combinedPatch);
|
||||
} catch {
|
||||
changesApplied = false;
|
||||
const patchTexts = await Promise.all(
|
||||
nonEmptyPatches.map(async patchPath => Bun.file(patchPath).text()),
|
||||
);
|
||||
const combinedPatch = patchTexts
|
||||
.map(text => (text.endsWith("\n") ? text : `${text}\n`))
|
||||
.join("");
|
||||
if (!combinedPatch.trim()) {
|
||||
changesApplied = true;
|
||||
} else {
|
||||
changesApplied = await git.patch.canApplyText(repoRoot, combinedPatch);
|
||||
if (changesApplied) {
|
||||
try {
|
||||
await git.patch.applyText(repoRoot, combinedPatch);
|
||||
} catch {
|
||||
changesApplied = false;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (changesApplied) {
|
||||
mergeSummary = "\n\nApplied patches: yes";
|
||||
} else {
|
||||
const notification =
|
||||
"<system-notification>Patches were not applied and must be handled manually.</system-notification>";
|
||||
const patchList =
|
||||
patchPaths.length > 0
|
||||
? `\n\nPatch artifacts:\n${patchPaths.map(patch => `- ${patch}`).join("\n")}`
|
||||
: "";
|
||||
mergeSummary = `\n\n${notification}${patchList}`;
|
||||
if (changesApplied) {
|
||||
mergeSummary = "\n\nApplied patches: yes";
|
||||
} else {
|
||||
const notification =
|
||||
"<system-notification>Patches were not applied and must be handled manually.</system-notification>";
|
||||
const patchList =
|
||||
patchPaths.length > 0
|
||||
? `\n\nPatch artifacts:\n${patchPaths.map(patch => `- ${patch}`).join("\n")}`
|
||||
: "";
|
||||
mergeSummary = `\n\n${notification}${patchList}`;
|
||||
}
|
||||
}
|
||||
} catch (mergeErr) {
|
||||
const msg = mergeErr instanceof Error ? mergeErr.message : String(mergeErr);
|
||||
changesApplied = false;
|
||||
mergeSummary = `\n\n<system-notification>Merge phase failed: ${msg}\nTask outputs are preserved but changes were not applied.</system-notification>`;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -515,29 +515,49 @@ export async function mergeTaskBranches(
|
||||
const merged: string[] = [];
|
||||
const failed: string[] = [];
|
||||
|
||||
for (const { branchName } of branches) {
|
||||
try {
|
||||
await git.cherryPick(repoRoot, branchName);
|
||||
} catch (err) {
|
||||
await git.cherryPick.abort(repoRoot);
|
||||
const stderr =
|
||||
err instanceof git.GitCommandError
|
||||
? err.result.stderr.trim()
|
||||
: err instanceof Error
|
||||
? err.message
|
||||
: String(err);
|
||||
failed.push(branchName);
|
||||
return {
|
||||
merged,
|
||||
failed: [...failed, ...branches.slice(merged.length + failed.length).map(b => b.branchName)],
|
||||
conflict: `${branchName}: ${stderr}`,
|
||||
};
|
||||
// Stash dirty working tree so cherry-pick can operate on a clean HEAD.
|
||||
// Without this, cherry-pick refuses to run when uncommitted changes exist.
|
||||
const didStash = await git.stash.push(repoRoot, "omp-task-merge");
|
||||
|
||||
try {
|
||||
for (const { branchName } of branches) {
|
||||
try {
|
||||
await git.cherryPick(repoRoot, branchName);
|
||||
} catch (err) {
|
||||
try {
|
||||
await git.cherryPick.abort(repoRoot);
|
||||
} catch {
|
||||
/* no state to abort */
|
||||
}
|
||||
const stderr =
|
||||
err instanceof git.GitCommandError
|
||||
? err.result.stderr.trim()
|
||||
: err instanceof Error
|
||||
? err.message
|
||||
: String(err);
|
||||
failed.push(branchName);
|
||||
return {
|
||||
merged,
|
||||
failed: [...failed, ...branches.slice(merged.length + failed.length).map(b => b.branchName)],
|
||||
conflict: `${branchName}: ${stderr}`,
|
||||
};
|
||||
}
|
||||
|
||||
merged.push(branchName);
|
||||
}
|
||||
|
||||
merged.push(branchName);
|
||||
return { merged, failed };
|
||||
} finally {
|
||||
if (didStash) {
|
||||
try {
|
||||
await git.stash.pop(repoRoot);
|
||||
} catch {
|
||||
// Pop can fail if cherry-picked changes conflict with stashed changes.
|
||||
// The stash entry is preserved — the user can resolve manually.
|
||||
logger.warn("Failed to restore stashed changes after task merge; stash entry preserved");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return { merged, failed };
|
||||
}
|
||||
|
||||
/** Clean up temporary task branches. */
|
||||
|
||||
Reference in New Issue
Block a user