diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index d47005cb7..f7bbafaca 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -9,6 +9,7 @@ ### Added - Added `fuse-overlay` isolation mode for subagents using `fuse-overlayfs` (copy-on-write overlay, no baseline patch apply needed) +- Added `task.isolation.merge` setting (`patch` or `branch`) to control how isolated task changes are integrated back. `branch` mode commits each task to a temp branch and merges with `--no-ff` for proper commit history ## [13.2.0] - 2026-02-23 ### Breaking Changes diff --git a/packages/coding-agent/DEVELOPMENT.md b/packages/coding-agent/DEVELOPMENT.md index 408477d72..cff1843f5 100644 --- a/packages/coding-agent/DEVELOPMENT.md +++ b/packages/coding-agent/DEVELOPMENT.md @@ -909,6 +909,9 @@ What _is_ isolated is execution context and artifacts, not process memory: - Optional filesystem isolation is controlled by the `task.isolation.mode` setting (`"none"`, `"worktree"`, or `"fuse-overlay"`). - **worktree**: `ensureWorktree(...)`, `applyBaseline(...)`, `captureDeltaPatch(...)`, `cleanupWorktree(...)`. - **fuse-overlay**: `ensureFuseOverlay(...)` (mounts a copy-on-write overlay via `fuse-overlayfs`), `captureDeltaPatch(...)`, `cleanupFuseOverlay(...)`. No baseline apply step needed since the overlay reflects the full working tree. +- The `task.isolation.merge` setting controls how isolated changes are integrated back: + - **patch** (default): captures a diff via `captureDeltaPatch(...)`, combines patches, and applies with `git apply`. + - **branch**: each task commits to a temp branch (`omp/task/`) via `commitToBranch(...)`, then `mergeTaskBranches(...)` merges them sequentially with `--no-ff` merge commits. - Child session JSONL/markdown outputs are written under the task artifacts directory (`.jsonl`, `.md`, and in isolated mode `.patch`). ### Tooling Surface in Child Sessions diff --git a/packages/coding-agent/src/config/settings-schema.ts b/packages/coding-agent/src/config/settings-schema.ts index a14363e07..b92dc7c53 100644 --- a/packages/coding-agent/src/config/settings-schema.ts +++ b/packages/coding-agent/src/config/settings-schema.ts @@ -555,6 +555,17 @@ export const SETTINGS_SCHEMA = { submenu: true, }, }, + "task.isolation.merge": { + type: "enum", + values: ["patch", "branch"] as const, + default: "patch", + ui: { + tab: "tools", + label: "Task isolation merge", + description: "How isolated task changes are integrated (patch apply or branch merge)", + submenu: true, + }, + }, "task.maxConcurrency": { type: "number", default: 32, diff --git a/packages/coding-agent/src/modes/components/settings-defs.ts b/packages/coding-agent/src/modes/components/settings-defs.ts index 6c1f5b39d..0dbbd40e6 100644 --- a/packages/coding-agent/src/modes/components/settings-defs.ts +++ b/packages/coding-agent/src/modes/components/settings-defs.ts @@ -93,6 +93,17 @@ const OPTION_PROVIDERS: Partial> = { { value: "2", label: "Double" }, { value: "3", label: "Triple" }, ], + // Task isolation mode + "task.isolation.mode": [ + { value: "none", label: "None", description: "No isolation" }, + { value: "worktree", label: "Worktree", description: "Git worktree isolation" }, + { value: "fuse-overlay", label: "Fuse Overlay", description: "COW overlay via fuse-overlayfs" }, + ], + // Task isolation merge strategy + "task.isolation.merge": [ + { value: "patch", label: "Patch", description: "Combine diffs and git apply" }, + { value: "branch", label: "Branch", description: "Commit per task, merge with --no-ff" }, + ], // Todo max reminders "todo.reminders.max": [ { value: "1", label: "1 reminder" }, diff --git a/packages/coding-agent/src/prompts/tools/task-summary.md b/packages/coding-agent/src/prompts/tools/task-summary.md index f8de1ac68..5082dc83e 100644 --- a/packages/coding-agent/src/prompts/tools/task-summary.md +++ b/packages/coding-agent/src/prompts/tools/task-summary.md @@ -20,9 +20,9 @@ {{/unless}} {{/each}} -{{#if patchApplySummary}} - -{{patchApplySummary}} - +{{#if mergeSummary}} + +{{mergeSummary}} + {{/if}} \ No newline at end of file diff --git a/packages/coding-agent/src/task/index.ts b/packages/coding-agent/src/task/index.ts index 2ed4b6b92..3808ba79d 100644 --- a/packages/coding-agent/src/task/index.ts +++ b/packages/coding-agent/src/task/index.ts @@ -47,13 +47,17 @@ import { } from "./types"; import { applyBaseline, + applyNestedPatches, captureBaseline, captureDeltaPatch, cleanupFuseOverlay, + cleanupTaskBranches, cleanupWorktree, + commitToBranch, ensureFuseOverlay, ensureWorktree, getRepoRoot, + mergeTaskBranches, type WorktreeBaseline, } from "./worktree"; @@ -427,6 +431,7 @@ export class TaskTool implements AgentTool { const isolationMode = this.session.settings.get("task.isolation.mode"); const isolationRequested = "isolated" in params ? params.isolated === true : false; const isIsolated = isolationMode !== "none" && isolationRequested; + const mergeMode = this.session.settings.get("task.isolation.merge"); const maxConcurrency = this.session.settings.get("task.maxConcurrency"); const taskDepth = this.session.taskDepth ?? 0; @@ -839,12 +844,21 @@ export class TaskTool implements AgentTool { preloadedSkills: task.preloadedSkills, promptTemplates, }); - const patch = await captureDeltaPatch(isolationDir, baseline); + if (mergeMode === "branch") { + const commitResult = await commitToBranch(isolationDir, baseline, task.id, task.description); + return { + ...result, + branchName: commitResult?.branchName, + nestedPatches: commitResult?.nestedPatches, + }; + } + const delta = await captureDeltaPatch(isolationDir, baseline); const patchPath = path.join(effectiveArtifactsDir, `${task.id}.patch`); - await Bun.write(patchPath, patch); + await Bun.write(patchPath, delta.rootPatch); return { ...result, patchPath, + nestedPatches: delta.nestedPatches, }; } catch (err) { const message = err instanceof Error ? err.message : String(err); @@ -930,64 +944,111 @@ export class TaskTool implements AgentTool { } } - let patchApplySummary = ""; - let patchesApplied: boolean | null = null; - if (isIsolated) { - const patchesInOrder = results.map(result => result.patchPath).filter(Boolean) as string[]; - const missingPatch = results.some(result => !result.patchPath); - if (!repoRoot || missingPatch) { - patchesApplied = 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) { - patchesApplied = true; + let mergeSummary = ""; + let changesApplied: boolean | 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 })); + + if (branchEntries.length === 0) { + changesApplied = true; } else { - 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()) { - patchesApplied = true; + const mergeResult = await mergeTaskBranches(repoRoot, branchEntries); + changesApplied = mergeResult.failed.length === 0; + + if (changesApplied) { + mergeSummary = `\n\nMerged ${mergeResult.merged.length} branch${mergeResult.merged.length === 1 ? "" : "es"}: ${mergeResult.merged.join(", ")}`; } else { - const combinedPatchPath = path.join(os.tmpdir(), `omp-task-combined-${Snowflake.next()}.patch`); - try { - await Bun.write(combinedPatchPath, combinedPatch); - const checkResult = await $`git apply --check --binary ${combinedPatchPath}` - .cwd(repoRoot) - .quiet() - .nothrow(); - if (checkResult.exitCode !== 0) { - patchesApplied = false; - } else { - const applyResult = await $`git apply --binary ${combinedPatchPath}` + 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\nBranch merge failed. ${mergedPart}${failedPart}${conflictPart}\nUnmerged branches remain for manual resolution.`; + } + } + + // 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) { + changesApplied = true; + } else { + 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 { + const combinedPatchPath = path.join(os.tmpdir(), `omp-task-combined-${Snowflake.next()}.patch`); + try { + await Bun.write(combinedPatchPath, combinedPatch); + const checkResult = await $`git apply --check --binary ${combinedPatchPath}` .cwd(repoRoot) .quiet() .nothrow(); - patchesApplied = applyResult.exitCode === 0; + if (checkResult.exitCode !== 0) { + changesApplied = false; + } else { + const applyResult = await $`git apply --binary ${combinedPatchPath}` + .cwd(repoRoot) + .quiet() + .nothrow(); + changesApplied = applyResult.exitCode === 0; + } + } finally { + await fs.rm(combinedPatchPath, { force: true }); } - } finally { - await fs.rm(combinedPatchPath, { force: true }); } } } - } - if (patchesApplied) { - patchApplySummary = "\n\nApplied patches: yes"; - } else { - const notification = - "Patches were not applied and must be handled manually."; - const patchList = - patchPaths.length > 0 - ? `\n\nPatch artifacts:\n${patchPaths.map(patch => `- ${patch}`).join("\n")}` - : ""; - patchApplySummary = `\n\n${notification}${patchList}`; + if (changesApplied) { + mergeSummary = "\n\nApplied patches: yes"; + } else { + const notification = + "Patches were not applied and must be handled manually."; + const patchList = + patchPaths.length > 0 + ? `\n\nPatch artifacts:\n${patchPaths.map(patch => `- ${patch}`).join("\n")}` + : ""; + mergeSummary = `\n\n${notification}${patchList}`; + } + } + } + + // Apply nested repo patches (separate from parent git) + if (isIsolated && repoRoot && changesApplied !== false) { + const allNestedPatches = results + .filter(r => r.nestedPatches && r.nestedPatches.length > 0 && r.exitCode === 0 && !r.aborted) + .flatMap(r => r.nestedPatches!); + if (allNestedPatches.length > 0) { + try { + await applyNestedPatches(repoRoot, allNestedPatches); + } catch { + // Nested patch failures are non-fatal to the parent merge + mergeSummary += + "\n\nSome nested repository patches failed to apply."; + } } } @@ -1034,12 +1095,12 @@ export class TaskTool implements AgentTool { summaries, outputIds, agentName, - patchApplySummary, + mergeSummary, }); // Cleanup temp directory if used const shouldCleanupTempArtifacts = - tempArtifactsDir && (!isIsolated || patchesApplied === true || patchesApplied === null); + tempArtifactsDir && (!isIsolated || changesApplied === true || changesApplied === null); if (shouldCleanupTempArtifacts) { await fs.rm(tempArtifactsDir, { recursive: true, force: true }); } diff --git a/packages/coding-agent/src/task/render.ts b/packages/coding-agent/src/task/render.ts index 2682f9db6..39f78f25c 100644 --- a/packages/coding-agent/src/task/render.ts +++ b/packages/coding-agent/src/task/render.ts @@ -847,6 +847,8 @@ function renderAgentResult(result: SingleResult, isLast: boolean, expanded: bool if (result.patchPath && !aborted && result.exitCode === 0) { lines.push(`${continuePrefix}${theme.fg("dim", `Patch: ${result.patchPath}`)}`); + } else if (result.branchName && !aborted && result.exitCode === 0) { + lines.push(`${continuePrefix}${theme.fg("dim", `Branch: ${result.branchName}`)}`); } // Error message diff --git a/packages/coding-agent/src/task/types.ts b/packages/coding-agent/src/task/types.ts index a726d799c..80ea0da33 100644 --- a/packages/coding-agent/src/task/types.ts +++ b/packages/coding-agent/src/task/types.ts @@ -2,6 +2,7 @@ import type { ThinkingLevel } from "@oh-my-pi/pi-agent-core"; import type { Usage } from "@oh-my-pi/pi-ai"; import { $env } from "@oh-my-pi/pi-utils"; import { type Static, Type } from "@sinclair/typebox"; +import type { NestedRepoPatch } from "./worktree"; /** Source of an agent definition */ export type AgentSource = "bundled" | "user" | "project"; @@ -179,6 +180,10 @@ export interface SingleResult { outputPath?: string; /** Patch path for isolated worktree output */ patchPath?: string; + /** Branch name for isolated branch-mode output */ + branchName?: string; + /** Nested repo patches to apply after parent merge */ + nestedPatches?: NestedRepoPatch[]; /** Data extracted by registered subprocess tool handlers (keyed by tool name) */ extractedToolData?: Record; /** Output metadata for agent:// URL integration */ diff --git a/packages/coding-agent/src/task/worktree.ts b/packages/coding-agent/src/task/worktree.ts index 104e352b6..4457be7f3 100644 --- a/packages/coding-agent/src/task/worktree.ts +++ b/packages/coding-agent/src/task/worktree.ts @@ -9,6 +9,7 @@ import { $ } from "bun"; /** Baseline state for a single git repository. */ export interface RepoBaseline { repoRoot: string; + headCommit: string; staged: string; unstaged: string; untracked: string[]; @@ -97,6 +98,7 @@ async function discoverNestedRepos(repoRoot: string): Promise { } async function captureRepoBaseline(repoRoot: string): Promise { + const headCommit = (await $`git rev-parse HEAD`.cwd(repoRoot).quiet().text()).trim(); const staged = await $`git diff --cached --binary`.cwd(repoRoot).quiet().text(); const unstaged = await $`git diff --binary`.cwd(repoRoot).quiet().text(); const untrackedRaw = await $`git ls-files --others --exclude-standard`.cwd(repoRoot).quiet().text(); @@ -104,7 +106,7 @@ async function captureRepoBaseline(repoRoot: string): Promise { .split("\n") .map(line => line.trim()) .filter(line => line.length > 0); - return { repoRoot, staged, unstaged, untracked }; + return { repoRoot, headCommit, staged, unstaged, untracked }; } export async function captureBaseline(repoRoot: string): Promise { @@ -204,9 +206,48 @@ async function listUntracked(cwd: string): Promise { } async function captureRepoDeltaPatch(repoDir: string, rb: RepoBaseline): Promise { + // Check if HEAD advanced (task committed changes) + const currentHead = (await $`git rev-parse HEAD`.cwd(repoDir).quiet().nothrow().text()).trim(); + const headAdvanced = currentHead && currentHead !== rb.headCommit; + + if (headAdvanced) { + // HEAD moved: use diff-tree to capture committed changes, plus any uncommitted on top + const parts: string[] = []; + + // Committed changes since baseline + const committedDiff = await $`git diff-tree -r -p --binary ${rb.headCommit} ${currentHead}` + .cwd(repoDir) + .quiet() + .nothrow() + .text(); + if (committedDiff.trim()) parts.push(committedDiff); + + // Uncommitted changes on top of the new HEAD + const staged = await $`git diff --cached --binary`.cwd(repoDir).quiet().text(); + const unstaged = await $`git diff --binary`.cwd(repoDir).quiet().text(); + if (staged.trim()) parts.push(staged); + if (unstaged.trim()) parts.push(unstaged); + + // New untracked files (relative to both baseline and current tracking) + const currentUntracked = await listUntracked(repoDir); + const baselineUntracked = new Set(rb.untracked); + const newUntracked = currentUntracked.filter(entry => !baselineUntracked.has(entry)); + if (newUntracked.length > 0) { + const untrackedDiffs = await Promise.all( + newUntracked.map(entry => + $`git diff --binary --no-index /dev/null ${entry}`.cwd(repoDir).quiet().nothrow().text(), + ), + ); + parts.push(...untrackedDiffs.filter(d => d.trim())); + } + + return parts.join("\n"); + } + + // HEAD unchanged: use temp index approach (subtracts baseline from delta) const tempIndex = path.join(os.tmpdir(), `omp-task-index-${Snowflake.next()}`); try { - await $`git read-tree HEAD`.cwd(repoDir).env({ GIT_INDEX_FILE: tempIndex }); + await $`git read-tree ${rb.headCommit}`.cwd(repoDir).env({ GIT_INDEX_FILE: tempIndex }); await applyPatchToIndex(repoDir, rb.staged, tempIndex); await applyPatchToIndex(repoDir, rb.unstaged, tempIndex); const diff = await $`git diff --binary`.cwd(repoDir).env({ GIT_INDEX_FILE: tempIndex }).quiet().text(); @@ -228,30 +269,46 @@ async function captureRepoDeltaPatch(repoDir: string, rb: RepoBaseline): Promise } } -/** Rewrite a/b paths in a unified diff to be prefixed with a subdirectory. */ -function prefixPatchPaths(patch: string, prefix: string): string { - if (!patch.trim()) return patch; - return patch.replace(/^(---| \+\+\+) (a|b)\//gm, (_, marker, ab) => `${marker} ${ab}/${prefix}/`); +export interface NestedRepoPatch { + relativePath: string; + patch: string; } -export async function captureDeltaPatch(isolationDir: string, baseline: WorktreeBaseline): Promise { +export interface DeltaPatchResult { + rootPatch: string; + nestedPatches: NestedRepoPatch[]; +} + +export async function captureDeltaPatch(isolationDir: string, baseline: WorktreeBaseline): Promise { const rootPatch = await captureRepoDeltaPatch(isolationDir, baseline.root); - const parts = [rootPatch]; + const nestedPatches: NestedRepoPatch[] = []; for (const { relativePath, baseline: nb } of baseline.nested) { const nestedDir = path.join(isolationDir, relativePath); try { await fs.access(path.join(nestedDir, ".git")); } catch { - continue; // nested repo doesn't exist in isolation dir - } - const nestedPatch = await captureRepoDeltaPatch(nestedDir, nb); - if (nestedPatch.trim()) { - parts.push(prefixPatchPaths(nestedPatch, relativePath)); + continue; } + const patch = await captureRepoDeltaPatch(nestedDir, nb); + if (patch.trim()) nestedPatches.push({ relativePath, patch }); } - return parts.filter(p => p.trim()).join("\n"); + return { rootPatch, nestedPatches }; +} + +/** Apply nested repo patches directly to their working directories after parent merge. */ +export async function applyNestedPatches(repoRoot: string, patches: NestedRepoPatch[]): Promise { + for (const { relativePath, patch } of patches) { + if (!patch.trim()) continue; + const nestedDir = path.join(repoRoot, relativePath); + try { + await fs.access(path.join(nestedDir, ".git")); + } catch { + continue; + } + await applyPatch(nestedDir, patch); + } } export async function cleanupWorktree(dir: string): Promise { @@ -328,47 +385,35 @@ export async function cleanupFuseOverlay(mergedDir: string): Promise { // Branch-mode isolation // ═══════════════════════════════════════════════════════════════════════════ +export interface CommitToBranchResult { + branchName?: string; + nestedPatches: NestedRepoPatch[]; +} + /** * Commit task-only changes to a new branch. - * Uses captureDeltaPatch to isolate the task's changes from the baseline, - * then applies that patch on a clean branch from HEAD. - * Returns the branch name, or null if no changes to commit. + * Only root repo changes go on the branch. Nested repo patches are returned + * separately since the parent git can't track files inside gitlinks. */ export async function commitToBranch( isolationDir: string, baseline: WorktreeBaseline, taskId: string, description: string | undefined, -): Promise { - // Capture root patch and nested patches separately - const rootPatch = await captureRepoDeltaPatch(isolationDir, baseline.root); - const nestedChanges: Array<{ relativePath: string; patch: string }> = []; - for (const { relativePath, baseline: nb } of baseline.nested) { - const nestedDir = path.join(isolationDir, relativePath); - try { - await fs.access(path.join(nestedDir, ".git")); - } catch { - continue; - } - const np = await captureRepoDeltaPatch(nestedDir, nb); - if (np.trim()) nestedChanges.push({ relativePath, patch: np }); - } - - const hasChanges = rootPatch.trim() || nestedChanges.length > 0; - if (!hasChanges) return null; +): Promise { + const { rootPatch, nestedPatches } = await captureDeltaPatch(isolationDir, baseline); + if (!rootPatch.trim() && nestedPatches.length === 0) return null; const repoRoot = baseline.root.repoRoot; const branchName = `omp/task/${taskId}`; const commitMessage = description || taskId; - await $`git branch ${branchName} HEAD`.cwd(repoRoot).quiet(); - - const tmpDir = path.join(os.tmpdir(), `omp-branch-${Snowflake.next()}`); - try { - await $`git worktree add ${tmpDir} ${branchName}`.cwd(repoRoot).quiet(); - - // Apply root repo patch via git apply - if (rootPatch.trim()) { + // Only create a branch if the root repo has changes + if (rootPatch.trim()) { + await $`git branch ${branchName} HEAD`.cwd(repoRoot).quiet(); + const tmpDir = path.join(os.tmpdir(), `omp-branch-${Snowflake.next()}`); + try { + await $`git worktree add ${tmpDir} ${branchName}`.cwd(repoRoot).quiet(); const patchPath = path.join(os.tmpdir(), `omp-branch-patch-${Snowflake.next()}.patch`); try { await Bun.write(patchPath, rootPatch); @@ -376,23 +421,15 @@ export async function commitToBranch( } finally { await fs.rm(patchPath, { force: true }); } + await $`git add -A`.cwd(tmpDir).quiet(); + await $`git -c user.name=omp -c user.email=omp@task commit -m ${commitMessage}`.cwd(tmpDir).quiet(); + } finally { + await $`git worktree remove -f ${tmpDir}`.cwd(repoRoot).quiet().nothrow(); + await fs.rm(tmpDir, { recursive: true, force: true }); } - - // Copy nested repo changes directly (they aren't tracked by root git) - for (const { relativePath } of nestedChanges) { - const nestedSrc = path.join(isolationDir, relativePath); - const nestedDst = path.join(tmpDir, relativePath); - await fs.cp(nestedSrc, nestedDst, { recursive: true }); - } - - await $`git add -A`.cwd(tmpDir).quiet(); - await $`git -c user.name=omp -c user.email=omp@task commit -m ${commitMessage}`.cwd(tmpDir).quiet(); - } finally { - await $`git worktree remove -f ${tmpDir}`.cwd(repoRoot).quiet().nothrow(); - await fs.rm(tmpDir, { recursive: true, force: true }); } - return branchName; + return { branchName: rootPatch.trim() ? branchName : undefined, nestedPatches }; } export interface MergeBranchResult {