From 86cd66b3d5b78c3f0393db2ba7b2fccfb05d3aef Mon Sep 17 00:00:00 2001 From: DeprecatedLuke <16418011+DeprecatedLuke@users.noreply.github.com> Date: Mon, 23 Feb 2026 16:41:11 +0000 Subject: [PATCH 1/7] feat(task): add fuse-overlay isolation backend --- packages/coding-agent/CHANGELOG.md | 8 +++ packages/coding-agent/DEVELOPMENT.md | 4 +- .../src/config/settings-schema.ts | 9 +-- packages/coding-agent/src/config/settings.ts | 10 ++++ .../coding-agent/src/prompts/tools/task.md | 2 +- packages/coding-agent/src/task/index.ts | 43 +++++++++----- packages/coding-agent/src/task/types.ts | 2 +- packages/coding-agent/src/task/worktree.ts | 56 +++++++++++++++++++ 8 files changed, 112 insertions(+), 22 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 11f8dcc54..d47005cb7 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,14 @@ ## [Unreleased] +### Breaking Changes + +- Renamed `task.isolation.enabled` (boolean) setting to `task.isolation.mode` (enum: `none`, `worktree`, `fuse-overlay`). Existing `true`/`false` values are auto-migrated to `worktree`/`none`. + +### Added + +- Added `fuse-overlay` isolation mode for subagents using `fuse-overlayfs` (copy-on-write overlay, no baseline patch apply needed) + ## [13.2.0] - 2026-02-23 ### Breaking Changes diff --git a/packages/coding-agent/DEVELOPMENT.md b/packages/coding-agent/DEVELOPMENT.md index 4698000b0..408477d72 100644 --- a/packages/coding-agent/DEVELOPMENT.md +++ b/packages/coding-agent/DEVELOPMENT.md @@ -906,7 +906,9 @@ Despite the name `runSubprocess`, `packages/coding-agent/src/task/executor.ts` c What _is_ isolated is execution context and artifacts, not process memory: -- Optional git worktree isolation is handled by `TaskTool.execute(...)` in `index.ts` using `ensureWorktree(...)`, `applyBaseline(...)`, `captureDeltaPatch(...)`, `cleanupWorktree(...)`. +- 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. - 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 1177433f1..a14363e07 100644 --- a/packages/coding-agent/src/config/settings-schema.ts +++ b/packages/coding-agent/src/config/settings-schema.ts @@ -544,13 +544,14 @@ export const SETTINGS_SCHEMA = { // ───────────────────────────────────────────────────────────────────────── // Task tool settings // ───────────────────────────────────────────────────────────────────────── - "task.isolation.enabled": { - type: "boolean", - default: false, + "task.isolation.mode": { + type: "enum", + values: ["none", "worktree", "fuse-overlay"] as const, + default: "none", ui: { tab: "tools", label: "Task isolation", - description: "Run subagents in isolated git worktrees", + description: "Isolation mode for subagents (none, git worktree, or fuse-overlay)", submenu: true, }, }, diff --git a/packages/coding-agent/src/config/settings.ts b/packages/coding-agent/src/config/settings.ts index 95fb49615..54d761108 100644 --- a/packages/coding-agent/src/config/settings.ts +++ b/packages/coding-agent/src/config/settings.ts @@ -546,6 +546,16 @@ export class Settings { } } + // task.isolation.enabled (boolean) -> task.isolation.mode (enum) + const taskObj = raw.task as Record | undefined; + const isolationObj = taskObj?.isolation as Record | undefined; + if (isolationObj && "enabled" in isolationObj) { + if (typeof isolationObj.enabled === "boolean") { + isolationObj.mode = isolationObj.enabled ? "worktree" : "none"; + } + delete isolationObj.enabled; + } + return raw; } diff --git a/packages/coding-agent/src/prompts/tools/task.md b/packages/coding-agent/src/prompts/tools/task.md index 03e22c22e..73d2546ef 100644 --- a/packages/coding-agent/src/prompts/tools/task.md +++ b/packages/coding-agent/src/prompts/tools/task.md @@ -18,7 +18,7 @@ Subagents lack your conversation history. Every decision, file content, and user - `context`: Shared background prepended to every assignment. Session-specific info only. - `schema`: JTD schema for expected output. Format lives here — MUST NOT be duplicated in assignments. - `tasks`: Tasks to execute in parallel. -- `isolated`: Run in isolated git worktree; returns patches. Use when tasks edit overlapping files. +- `isolated`: Run in isolated environment; returns patches. Use when tasks edit overlapping files. diff --git a/packages/coding-agent/src/task/index.ts b/packages/coding-agent/src/task/index.ts index 88c073b21..2ed4b6b92 100644 --- a/packages/coding-agent/src/task/index.ts +++ b/packages/coding-agent/src/task/index.ts @@ -49,7 +49,9 @@ import { applyBaseline, captureBaseline, captureDeltaPatch, + cleanupFuseOverlay, cleanupWorktree, + ensureFuseOverlay, ensureWorktree, getRepoRoot, type WorktreeBaseline, @@ -145,11 +147,11 @@ export class TaskTool implements AgentTool { get description(): string { const disabledAgents = this.session.settings.get("task.disabledAgents") as string[]; const maxConcurrency = this.session.settings.get("task.maxConcurrency"); - const isolationEnabled = this.session.settings.get("task.isolation.enabled"); + const isolationMode = this.session.settings.get("task.isolation.mode"); return renderDescription( this.#discoveredAgents, maxConcurrency, - isolationEnabled, + isolationMode !== "none", this.session.settings.get("async.enabled"), disabledAgents, ); @@ -168,9 +170,9 @@ export class TaskTool implements AgentTool { * Create a TaskTool instance with async agent discovery. */ static async create(session: ToolSession): Promise { - const isolationEnabled = session.settings.get("task.isolation.enabled"); + const isolationMode = session.settings.get("task.isolation.mode"); const { agents } = await discoverAgents(session.cwd); - return new TaskTool(session, agents, isolationEnabled); + return new TaskTool(session, agents, isolationMode !== "none"); } async execute( @@ -422,18 +424,18 @@ export class TaskTool implements AgentTool { const startTime = Date.now(); const { agents, projectAgentsDir } = await discoverAgents(this.session.cwd); const { agent: agentName, context, schema: outputSchema } = params; - const isolationEnabled = this.session.settings.get("task.isolation.enabled"); + const isolationMode = this.session.settings.get("task.isolation.mode"); const isolationRequested = "isolated" in params ? params.isolated === true : false; - const isIsolated = isolationEnabled && isolationRequested; + const isIsolated = isolationMode !== "none" && isolationRequested; const maxConcurrency = this.session.settings.get("task.maxConcurrency"); const taskDepth = this.session.taskDepth ?? 0; - if (!isolationEnabled && "isolated" in params) { + if (isolationMode === "none" && "isolated" in params) { return { content: [ { type: "text", - text: "Task isolation is disabled. Remove the isolated argument to run subagents.", + text: "Task isolation is disabled. Remove the isolated argument or set task.isolation.mode to 'worktree' or 'fuse-overlay'.", }, ], details: { @@ -789,16 +791,23 @@ export class TaskTool implements AgentTool { } const taskStart = Date.now(); - let worktreeDir: string | undefined; + let isolationDir: string | undefined; try { if (!repoRoot || !baseline) { throw new Error("Isolated task execution not initialized."); } - worktreeDir = await ensureWorktree(repoRoot, task.id); - await applyBaseline(worktreeDir, baseline); + + if (isolationMode === "fuse-overlay") { + isolationDir = await ensureFuseOverlay(repoRoot, task.id); + // Overlay already reflects the full working tree state — no baseline apply needed + } else { + isolationDir = await ensureWorktree(repoRoot, task.id); + await applyBaseline(isolationDir, baseline); + } + const result = await runSubprocess({ cwd: this.session.cwd, - worktree: worktreeDir, + worktree: isolationDir, agent, task: task.task, description: task.description, @@ -830,7 +839,7 @@ export class TaskTool implements AgentTool { preloadedSkills: task.preloadedSkills, promptTemplates, }); - const patch = await captureDeltaPatch(worktreeDir, baseline); + const patch = await captureDeltaPatch(isolationDir, baseline); const patchPath = path.join(effectiveArtifactsDir, `${task.id}.patch`); await Bun.write(patchPath, patch); return { @@ -856,8 +865,12 @@ export class TaskTool implements AgentTool { error: message, }; } finally { - if (worktreeDir) { - await cleanupWorktree(worktreeDir); + if (isolationDir) { + if (isolationMode === "fuse-overlay") { + await cleanupFuseOverlay(isolationDir); + } else { + await cleanupWorktree(isolationDir); + } } } }; diff --git a/packages/coding-agent/src/task/types.ts b/packages/coding-agent/src/task/types.ts index fbe6d43cd..a726d799c 100644 --- a/packages/coding-agent/src/task/types.ts +++ b/packages/coding-agent/src/task/types.ts @@ -77,7 +77,7 @@ const createTaskSchema = (options: { isolationEnabled: boolean }) => { ...properties, isolated: Type.Optional( Type.Boolean({ - description: "Run in isolated git worktree; returns patches. Use when tasks edit overlapping files.", + description: "Run in isolated environment; returns patches. Use when tasks edit overlapping files.", }), ), }); diff --git a/packages/coding-agent/src/task/worktree.ts b/packages/coding-agent/src/task/worktree.ts index 33570ffc8..433047e0b 100644 --- a/packages/coding-agent/src/task/worktree.ts +++ b/packages/coding-agent/src/task/worktree.ts @@ -168,3 +168,59 @@ export async function cleanupWorktree(dir: string): Promise { await fs.rm(dir, { recursive: true, force: true }); } } + +// ═══════════════════════════════════════════════════════════════════════════ +// Fuse-overlay isolation +// ═══════════════════════════════════════════════════════════════════════════ + +export async function ensureFuseOverlay(baseCwd: string, id: string): Promise { + const repoRoot = await getRepoRoot(baseCwd); + const encodedProject = getEncodedProjectName(repoRoot); + const baseDir = getWorktreeDir(encodedProject, id); + const upperDir = path.join(baseDir, "upper"); + const workDir = path.join(baseDir, "work"); + const mergedDir = path.join(baseDir, "merged"); + + // Clean up any stale mount at this path + const fusermount = Bun.which("fusermount3") ?? Bun.which("fusermount"); + if (fusermount) { + await $`${fusermount} -u ${mergedDir}`.quiet().nothrow(); + } + await fs.rm(baseDir, { recursive: true, force: true }); + + await fs.mkdir(upperDir, { recursive: true }); + await fs.mkdir(workDir, { recursive: true }); + await fs.mkdir(mergedDir, { recursive: true }); + + const binary = Bun.which("fuse-overlayfs"); + if (!binary) { + await fs.rm(baseDir, { recursive: true, force: true }); + throw new Error( + "fuse-overlayfs not found. Install it (e.g. `apt install fuse-overlayfs` or `pacman -S fuse-overlayfs`) to use fuse-overlay isolation.", + ); + } + + const result = await $`${binary} -o lowerdir=${repoRoot},upperdir=${upperDir},workdir=${workDir} ${mergedDir}` + .quiet() + .nothrow(); + if (result.exitCode !== 0) { + const stderr = result.stderr.toString().trim(); + await fs.rm(baseDir, { recursive: true, force: true }); + throw new Error(`fuse-overlayfs mount failed (exit ${result.exitCode}): ${stderr}`); + } + + return mergedDir; +} + +export async function cleanupFuseOverlay(mergedDir: string): Promise { + try { + const fusermount = Bun.which("fusermount3") ?? Bun.which("fusermount"); + if (fusermount) { + await $`${fusermount} -u ${mergedDir}`.quiet().nothrow(); + } + } finally { + // baseDir is the parent of the merged directory + const baseDir = path.dirname(mergedDir); + await fs.rm(baseDir, { recursive: true, force: true }); + } +} From 995b26b90fec0041e8fbff9531c6126d5ae3c76d Mon Sep 17 00:00:00 2001 From: DeprecatedLuke <16418011+DeprecatedLuke@users.noreply.github.com> Date: Mon, 23 Feb 2026 17:35:11 +0000 Subject: [PATCH 2/7] feat(task): add nested non-submodule repo support for isolation --- packages/coding-agent/src/task/worktree.ts | 278 ++++++++++++++++++--- 1 file changed, 248 insertions(+), 30 deletions(-) diff --git a/packages/coding-agent/src/task/worktree.ts b/packages/coding-agent/src/task/worktree.ts index 433047e0b..104e352b6 100644 --- a/packages/coding-agent/src/task/worktree.ts +++ b/packages/coding-agent/src/task/worktree.ts @@ -1,3 +1,4 @@ +import type { Dirent } from "node:fs"; import * as fs from "node:fs/promises"; import * as os from "node:os"; import path from "node:path"; @@ -5,13 +6,21 @@ import { isEnoent, Snowflake } from "@oh-my-pi/pi-utils"; import { getWorktreeDir } from "@oh-my-pi/pi-utils/dirs"; import { $ } from "bun"; -export interface WorktreeBaseline { +/** Baseline state for a single git repository. */ +export interface RepoBaseline { repoRoot: string; staged: string; unstaged: string; untracked: string[]; } +/** Baseline state for the project, including any nested git repos. */ +export interface WorktreeBaseline { + root: RepoBaseline; + /** Nested git repos (path relative to root.repoRoot). */ + nested: Array<{ relativePath: string; baseline: RepoBaseline }>; +} + export function getEncodedProjectName(cwd: string): string { return `--${cwd.replace(/^[/\\]/, "").replace(/[/\\:]/g, "-")}--`; } @@ -39,7 +48,55 @@ export async function ensureWorktree(baseCwd: string, id: string): Promise { +/** Find nested git repositories (non-submodule) under the given root. */ +async function discoverNestedRepos(repoRoot: string): Promise { + // Get submodule paths so we can exclude them + const submoduleRaw = await $`git submodule --quiet foreach --recursive 'echo $sm_path'` + .cwd(repoRoot) + .quiet() + .nothrow() + .text(); + const submodulePaths = new Set( + submoduleRaw + .split("\n") + .map(l => l.trim()) + .filter(Boolean), + ); + + // Find all .git dirs/files that aren't the root or known submodules + const result: string[] = []; + async function walk(dir: string): Promise { + let entries: Dirent[]; + try { + entries = await fs.readdir(dir, { withFileTypes: true }); + } catch { + return; + } + for (const entry of entries) { + if (entry.name === "node_modules" || entry.name === ".git") continue; + if (!entry.isDirectory()) continue; + const full = path.join(dir, entry.name); + const rel = path.relative(repoRoot, full); + // Check if this directory is itself a git repo + const gitDir = path.join(full, ".git"); + let hasGit = false; + try { + await fs.access(gitDir); + hasGit = true; + } catch {} + if (hasGit && !submodulePaths.has(rel)) { + result.push(rel); + // Don't recurse into nested repos — they manage their own tree + continue; + } + await walk(full); + } + } + await walk(repoRoot); + return result; +} + +async function captureRepoBaseline(repoRoot: string): Promise { 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(); @@ -47,13 +104,18 @@ export async function captureBaseline(repoRoot: string): Promise line.trim()) .filter(line => line.length > 0); + return { repoRoot, staged, unstaged, untracked }; +} - return { - repoRoot, - staged, - unstaged, - untracked, - }; +export async function captureBaseline(repoRoot: string): Promise { + const [root, nestedPaths] = await Promise.all([captureRepoBaseline(repoRoot), discoverNestedRepos(repoRoot)]); + const nested = await Promise.all( + nestedPaths.map(async relativePath => ({ + relativePath, + baseline: await captureRepoBaseline(path.join(repoRoot, relativePath)), + })), + ); + return { root, nested }; } async function writeTempPatchFile(patch: string): Promise { @@ -81,13 +143,13 @@ async function applyPatch( } } -export async function applyBaseline(worktreeDir: string, baseline: WorktreeBaseline): Promise { - await applyPatch(worktreeDir, baseline.staged, { cached: true }); - await applyPatch(worktreeDir, baseline.staged); - await applyPatch(worktreeDir, baseline.unstaged); +async function applyRepoBaseline(worktreeDir: string, rb: RepoBaseline, sourceRoot: string): Promise { + await applyPatch(worktreeDir, rb.staged, { cached: true }); + await applyPatch(worktreeDir, rb.staged); + await applyPatch(worktreeDir, rb.unstaged); - for (const entry of baseline.untracked) { - const source = path.join(baseline.repoRoot, entry); + for (const entry of rb.untracked) { + const source = path.join(sourceRoot, entry); const destination = path.join(worktreeDir, entry); try { await fs.mkdir(path.dirname(destination), { recursive: true }); @@ -99,6 +161,25 @@ export async function applyBaseline(worktreeDir: string, baseline: WorktreeBasel } } +export async function applyBaseline(worktreeDir: string, baseline: WorktreeBaseline): Promise { + await applyRepoBaseline(worktreeDir, baseline.root, baseline.root.repoRoot); + + // Restore nested repos into the worktree + for (const { relativePath, baseline: nb } of baseline.nested) { + const nestedDir = path.join(worktreeDir, relativePath); + // Copy the nested repo wholesale (it's not managed by root git) + const sourceDir = path.join(baseline.root.repoRoot, relativePath); + try { + await fs.cp(sourceDir, nestedDir, { recursive: true }); + } catch (err) { + if (isEnoent(err)) continue; + throw err; + } + // Then apply any uncommitted changes from the nested baseline + await applyRepoBaseline(nestedDir, nb, nb.repoRoot); + } +} + async function applyPatchToIndex(cwd: string, patch: string, indexFile: string): Promise { if (!patch.trim()) return; const tempPath = await writeTempPatchFile(patch); @@ -122,31 +203,23 @@ async function listUntracked(cwd: string): Promise { .filter(line => line.length > 0); } -export async function captureDeltaPatch(worktreeDir: string, baseline: WorktreeBaseline): Promise { +async function captureRepoDeltaPatch(repoDir: string, rb: RepoBaseline): Promise { const tempIndex = path.join(os.tmpdir(), `omp-task-index-${Snowflake.next()}`); try { - await $`git read-tree HEAD`.cwd(worktreeDir).env({ - GIT_INDEX_FILE: tempIndex, - }); - await applyPatchToIndex(worktreeDir, baseline.staged, tempIndex); - await applyPatchToIndex(worktreeDir, baseline.unstaged, tempIndex); - const diff = await $`git diff --binary` - .cwd(worktreeDir) - .env({ - GIT_INDEX_FILE: tempIndex, - }) - .quiet() - .text(); + await $`git read-tree HEAD`.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(); - const currentUntracked = await listUntracked(worktreeDir); - const baselineUntracked = new Set(baseline.untracked); + const currentUntracked = await listUntracked(repoDir); + const baselineUntracked = new Set(rb.untracked); const newUntracked = currentUntracked.filter(entry => !baselineUntracked.has(entry)); if (newUntracked.length === 0) return diff; const untrackedDiffs = await Promise.all( newUntracked.map(entry => - $`git diff --binary --no-index /dev/null ${entry}`.cwd(worktreeDir).quiet().nothrow().text(), + $`git diff --binary --no-index /dev/null ${entry}`.cwd(repoDir).quiet().nothrow().text(), ), ); return `${diff}${diff && !diff.endsWith("\n") ? "\n" : ""}${untrackedDiffs.join("\n")}`; @@ -155,6 +228,32 @@ export async function captureDeltaPatch(worktreeDir: string, baseline: WorktreeB } } +/** 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 async function captureDeltaPatch(isolationDir: string, baseline: WorktreeBaseline): Promise { + const rootPatch = await captureRepoDeltaPatch(isolationDir, baseline.root); + const parts = [rootPatch]; + + 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)); + } + } + + return parts.filter(p => p.trim()).join("\n"); +} + export async function cleanupWorktree(dir: string): Promise { try { const commonDirRaw = await $`git rev-parse --git-common-dir`.cwd(dir).quiet().nothrow().text(); @@ -224,3 +323,122 @@ export async function cleanupFuseOverlay(mergedDir: string): Promise { await fs.rm(baseDir, { recursive: true, force: true }); } } + +// ═══════════════════════════════════════════════════════════════════════════ +// Branch-mode isolation +// ═══════════════════════════════════════════════════════════════════════════ + +/** + * 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. + */ +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; + + 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()) { + const patchPath = path.join(os.tmpdir(), `omp-branch-patch-${Snowflake.next()}.patch`); + try { + await Bun.write(patchPath, rootPatch); + await $`git apply --binary ${patchPath}`.cwd(tmpDir).quiet(); + } finally { + await fs.rm(patchPath, { 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; +} + +export interface MergeBranchResult { + merged: string[]; + failed: string[]; + conflict?: string; +} + +/** + * Merge task branches sequentially into the working tree. + * Each branch gets a --no-ff merge commit preserving the task identity. + * Stops on first conflict and reports which branches succeeded. + */ +export async function mergeTaskBranches( + repoRoot: string, + branches: Array<{ branchName: string; taskId: string; description?: string }>, +): Promise { + const merged: string[] = []; + const failed: string[] = []; + + for (const { branchName, taskId, description } of branches) { + const mergeMessage = description || taskId; + + const result = await $`git merge --no-ff -m ${mergeMessage} ${branchName}`.cwd(repoRoot).quiet().nothrow(); + + if (result.exitCode !== 0) { + // Abort the failed merge to restore clean state + await $`git merge --abort`.cwd(repoRoot).quiet().nothrow(); + const stderr = result.stderr.toString().trim(); + failed.push(branchName); + return { + merged, + failed: [...failed, ...branches.slice(merged.length + failed.length).map(b => b.branchName)], + conflict: `${branchName}: ${stderr}`, + }; + } + + merged.push(branchName); + } + + return { merged, failed }; +} + +/** Clean up temporary task branches. */ +export async function cleanupTaskBranches(repoRoot: string, branches: string[]): Promise { + for (const branch of branches) { + await $`git branch -D ${branch}`.cwd(repoRoot).quiet().nothrow(); + } +} From c1370f7332578bce6862f1b646a3b9341bcf6ebe Mon Sep 17 00:00:00 2001 From: DeprecatedLuke <16418011+DeprecatedLuke@users.noreply.github.com> Date: Mon, 23 Feb 2026 17:35:19 +0000 Subject: [PATCH 3/7] feat(task): implement branch merge strategy for isolated tasks --- packages/coding-agent/CHANGELOG.md | 1 + packages/coding-agent/DEVELOPMENT.md | 3 + .../src/config/settings-schema.ts | 11 ++ .../src/modes/components/settings-defs.ts | 11 ++ .../src/prompts/tools/task-summary.md | 8 +- packages/coding-agent/src/task/index.ts | 165 ++++++++++++------ packages/coding-agent/src/task/render.ts | 2 + packages/coding-agent/src/task/types.ts | 5 + packages/coding-agent/src/task/worktree.ts | 149 ++++++++++------ 9 files changed, 243 insertions(+), 112 deletions(-) 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 { From 696d6ba545fdd00f7b811baf597112d85058b308 Mon Sep 17 00:00:00 2001 From: DeprecatedLuke <16418011+DeprecatedLuke@users.noreply.github.com> Date: Mon, 23 Feb 2026 19:10:54 +0000 Subject: [PATCH 4/7] feat(task): add better commit messages and nested repo isolation --- packages/coding-agent/CHANGELOG.md | 8 ++ .../src/config/settings-schema.ts | 11 ++ .../src/modes/components/settings-defs.ts | 5 + .../prompts/system/commit-message-system.md | 2 + packages/coding-agent/src/task/index.ts | 16 ++- packages/coding-agent/src/task/worktree.ts | 73 ++++++++++-- .../src/utils/commit-message-generator.ts | 108 ++++++++++++++++++ 7 files changed, 211 insertions(+), 12 deletions(-) create mode 100644 packages/coding-agent/src/prompts/system/commit-message-system.md create mode 100644 packages/coding-agent/src/utils/commit-message-generator.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index f7bbafaca..85ad5c984 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -10,6 +10,14 @@ - 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 +- Added `task.isolation.commits` setting (`generic` or `ai`) for nested repo commit messages. `ai` mode uses a smol model to generate conventional commit messages from diffs +- Nested non-submodule git repos are now discovered and handled during task isolation (changes captured and applied independently from parent repo) + +### Fixed + +- Fixed nested repo changes being lost when tasks commit inside the isolation (baseline state is now committed before task runs, so delta correctly excludes it) +- Fixed nested repo patches conflicting when multiple tasks contribute to the same repo (baseline untracked files no longer leak into patches) +- Nested repo changes are now committed after patch application (previously left as untracked files) ## [13.2.0] - 2026-02-23 ### Breaking Changes diff --git a/packages/coding-agent/src/config/settings-schema.ts b/packages/coding-agent/src/config/settings-schema.ts index b92dc7c53..71e485d71 100644 --- a/packages/coding-agent/src/config/settings-schema.ts +++ b/packages/coding-agent/src/config/settings-schema.ts @@ -566,6 +566,17 @@ export const SETTINGS_SCHEMA = { submenu: true, }, }, + "task.isolation.commits": { + type: "enum", + values: ["generic", "ai"] as const, + default: "generic", + ui: { + tab: "tools", + label: "Task isolation commits", + description: "Commit message style for nested repo changes (generic or AI-generated)", + 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 0dbbd40e6..1c9c6122d 100644 --- a/packages/coding-agent/src/modes/components/settings-defs.ts +++ b/packages/coding-agent/src/modes/components/settings-defs.ts @@ -104,6 +104,11 @@ const OPTION_PROVIDERS: Partial> = { { value: "patch", label: "Patch", description: "Combine diffs and git apply" }, { value: "branch", label: "Branch", description: "Commit per task, merge with --no-ff" }, ], + // Task isolation commit messages + "task.isolation.commits": [ + { value: "generic", label: "Generic", description: "Static commit message" }, + { value: "ai", label: "AI", description: "AI-generated commit message from diff" }, + ], // Todo max reminders "todo.reminders.max": [ { value: "1", label: "1 reminder" }, diff --git a/packages/coding-agent/src/prompts/system/commit-message-system.md b/packages/coding-agent/src/prompts/system/commit-message-system.md new file mode 100644 index 000000000..a91897b0b --- /dev/null +++ b/packages/coding-agent/src/prompts/system/commit-message-system.md @@ -0,0 +1,2 @@ +Generate a concise git commit message from the provided diff. Use conventional commit format: `type(scope): description` where type is feat/fix/refactor/chore/test/docs and scope is optional. The description MUST be lowercase, imperative mood, no trailing period. Keep it under 72 characters. +You MUST output ONLY the commit message, nothing else. diff --git a/packages/coding-agent/src/task/index.ts b/packages/coding-agent/src/task/index.ts index 3808ba79d..ec51ec1d3 100644 --- a/packages/coding-agent/src/task/index.ts +++ b/packages/coding-agent/src/task/index.ts @@ -29,6 +29,7 @@ import taskSummaryTemplate from "../prompts/tools/task-summary.md" with { type: import { formatBytes, formatDuration } from "../tools/render-utils"; // Import review tools for side effects (registers subagent tool handlers) import "../tools/review"; +import { generateCommitMessage } from "../utils/commit-message-generator"; import { discoverAgents, getAgent } from "./discovery"; import { runSubprocess } from "./executor"; import { AgentOutputManager } from "./output-manager"; @@ -432,6 +433,7 @@ export class TaskTool implements AgentTool { const isolationRequested = "isolated" in params ? params.isolated === true : false; const isIsolated = isolationMode !== "none" && isolationRequested; const mergeMode = this.session.settings.get("task.isolation.merge"); + const commitStyle = this.session.settings.get("task.isolation.commits"); const maxConcurrency = this.session.settings.get("task.maxConcurrency"); const taskDepth = this.session.taskDepth ?? 0; @@ -1043,7 +1045,19 @@ export class TaskTool implements AgentTool { .flatMap(r => r.nestedPatches!); if (allNestedPatches.length > 0) { try { - await applyNestedPatches(repoRoot, allNestedPatches); + const commitMsg = + commitStyle === "ai" && this.session.modelRegistry + ? async (diff: string) => { + const smolModel = this.session.settings.getModelRole("smol"); + return generateCommitMessage( + diff, + this.session.modelRegistry!, + smolModel, + this.session.getSessionId?.() ?? undefined, + ); + } + : undefined; + await applyNestedPatches(repoRoot, allNestedPatches, commitMsg); } catch { // Nested patch failures are non-fatal to the parent merge mergeSummary += diff --git a/packages/coding-agent/src/task/worktree.ts b/packages/coding-agent/src/task/worktree.ts index 4457be7f3..efb6256e7 100644 --- a/packages/coding-agent/src/task/worktree.ts +++ b/packages/coding-agent/src/task/worktree.ts @@ -6,6 +6,12 @@ import { isEnoent, Snowflake } from "@oh-my-pi/pi-utils"; import { getWorktreeDir } from "@oh-my-pi/pi-utils/dirs"; import { $ } from "bun"; +async function gitAuthorFlags(cwd: string): Promise { + const name = (await $`git config user.name`.cwd(cwd).quiet().nothrow().text()).trim() || "omp"; + const email = (await $`git config user.email`.cwd(cwd).quiet().nothrow().text()).trim() || "omp@task"; + return [`-c`, `user.name=${name}`, `-c`, `user.email=${email}`]; +} + /** Baseline state for a single git repository. */ export interface RepoBaseline { repoRoot: string; @@ -167,18 +173,33 @@ export async function applyBaseline(worktreeDir: string, baseline: WorktreeBasel await applyRepoBaseline(worktreeDir, baseline.root, baseline.root.repoRoot); // Restore nested repos into the worktree - for (const { relativePath, baseline: nb } of baseline.nested) { - const nestedDir = path.join(worktreeDir, relativePath); + for (const entry of baseline.nested) { + const nestedDir = path.join(worktreeDir, entry.relativePath); // Copy the nested repo wholesale (it's not managed by root git) - const sourceDir = path.join(baseline.root.repoRoot, relativePath); + const sourceDir = path.join(baseline.root.repoRoot, entry.relativePath); try { await fs.cp(sourceDir, nestedDir, { recursive: true }); } catch (err) { if (isEnoent(err)) continue; throw err; } - // Then apply any uncommitted changes from the nested baseline - await applyRepoBaseline(nestedDir, nb, nb.repoRoot); + // Apply any uncommitted changes from the nested baseline + await applyRepoBaseline(nestedDir, entry.baseline, entry.baseline.repoRoot); + // Commit baseline state so captureRepoDeltaPatch can cleanly subtract it. + // Without this, `git add -A && git commit` by the task would include + // baseline untracked files in the diff-tree output. + const hasChanges = (await $`git status --porcelain`.cwd(nestedDir).quiet().nothrow().text()).trim(); + if (hasChanges) { + await $`git add -A`.cwd(nestedDir).quiet(); + const flags = await gitAuthorFlags(nestedDir); + await $`git ${flags} commit -m omp-baseline --allow-empty`.cwd(nestedDir).quiet(); + // Update baseline to reflect the committed state — prevents double-apply + // in captureRepoDeltaPatch's temp-index path + entry.baseline.headCommit = (await $`git rev-parse HEAD`.cwd(nestedDir).quiet().text()).trim(); + entry.baseline.staged = ""; + entry.baseline.unstaged = ""; + entry.baseline.untracked = []; + } } } @@ -297,17 +318,46 @@ export async function captureDeltaPatch(isolationDir: string, baseline: Worktree 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; +/** + * Apply nested repo patches directly to their working directories after parent merge. + * @param commitMessage Optional async function to generate a commit message from the combined diff. + * If omitted or returns null, falls back to a generic message. + */ +export async function applyNestedPatches( + repoRoot: string, + patches: NestedRepoPatch[], + commitMessage?: (diff: string) => Promise, +): Promise { + // Group patches by target repo to apply all at once and commit + const byRepo = new Map(); + for (const p of patches) { + if (!p.patch.trim()) continue; + const group = byRepo.get(p.relativePath) ?? []; + group.push(p); + byRepo.set(p.relativePath, group); + } + + for (const [relativePath, repoPatches] of byRepo) { const nestedDir = path.join(repoRoot, relativePath); try { await fs.access(path.join(nestedDir, ".git")); } catch { continue; } - await applyPatch(nestedDir, patch); + + const combinedDiff = repoPatches.map(p => p.patch).join("\n"); + for (const { patch } of repoPatches) { + await applyPatch(nestedDir, patch); + } + + // Commit so nested repo history reflects the task changes + const hasChanges = (await $`git status --porcelain`.cwd(nestedDir).quiet().nothrow().text()).trim(); + if (hasChanges) { + const msg = (await commitMessage?.(combinedDiff)) ?? "changes from isolated task(s)"; + await $`git add -A`.cwd(nestedDir).quiet(); + const flags = await gitAuthorFlags(nestedDir); + await $`git ${flags} commit -m ${msg}`.cwd(nestedDir).quiet(); + } } } @@ -422,7 +472,8 @@ export async function commitToBranch( 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(); + const flags = await gitAuthorFlags(tmpDir); + await $`git ${flags} 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 }); diff --git a/packages/coding-agent/src/utils/commit-message-generator.ts b/packages/coding-agent/src/utils/commit-message-generator.ts new file mode 100644 index 000000000..71737c849 --- /dev/null +++ b/packages/coding-agent/src/utils/commit-message-generator.ts @@ -0,0 +1,108 @@ +/** + * Generate commit messages from diffs using a smol, fast model. + * Follows the same pattern as title-generator.ts. + */ +import type { Api, Model } from "@oh-my-pi/pi-ai"; +import { completeSimple } from "@oh-my-pi/pi-ai"; +import { logger } from "@oh-my-pi/pi-utils"; +import type { ModelRegistry } from "../config/model-registry"; +import { parseModelString } from "../config/model-resolver"; +import { renderPromptTemplate } from "../config/prompt-templates"; +import MODEL_PRIO from "../priority.json" with { type: "json" }; +import commitSystemPrompt from "../prompts/system/commit-message-system.md" with { type: "text" }; + +const COMMIT_SYSTEM_PROMPT = renderPromptTemplate(commitSystemPrompt); +const MAX_DIFF_CHARS = 4000; + +function getSmolModelCandidates(registry: ModelRegistry, savedSmolModel?: string): Model[] { + const availableModels = registry.getAvailable(); + if (availableModels.length === 0) return []; + + const candidates: Model[] = []; + const addCandidate = (model?: Model): void => { + if (!model) return; + if (candidates.some(c => c.provider === model.provider && c.id === model.id)) return; + candidates.push(model); + }; + + if (savedSmolModel) { + const parsed = parseModelString(savedSmolModel); + if (parsed) { + const match = availableModels.find(m => m.provider === parsed.provider && m.id === parsed.id); + addCandidate(match); + } + } + + for (const pattern of MODEL_PRIO.smol) { + const needle = pattern.toLowerCase(); + addCandidate(availableModels.find(m => m.id.toLowerCase() === needle)); + addCandidate(availableModels.find(m => m.id.toLowerCase().includes(needle))); + } + + for (const model of availableModels) { + addCandidate(model); + } + + return candidates; +} + +/** + * Generate a commit message from a unified diff. + * Returns null if generation fails (caller should fall back to generic message). + */ +export async function generateCommitMessage( + diff: string, + registry: ModelRegistry, + savedSmolModel?: string, + sessionId?: string, +): Promise { + const candidates = getSmolModelCandidates(registry, savedSmolModel); + if (candidates.length === 0) { + logger.debug("commit-msg-generator: no smol model found"); + return null; + } + + const truncatedDiff = diff.length > MAX_DIFF_CHARS ? `${diff.slice(0, MAX_DIFF_CHARS)}\n… (truncated)` : diff; + const userMessage = `\n${truncatedDiff}\n`; + + for (const model of candidates) { + const apiKey = await registry.getApiKey(model, sessionId); + if (!apiKey) continue; + + try { + const response = await completeSimple( + model, + { + systemPrompt: COMMIT_SYSTEM_PROMPT, + messages: [{ role: "user", content: userMessage, timestamp: Date.now() }], + }, + { apiKey, maxTokens: 60 }, + ); + + if (response.stopReason === "error") { + logger.debug("commit-msg-generator: error", { model: model.id, error: response.errorMessage }); + continue; + } + + let msg = ""; + for (const content of response.content) { + if (content.type === "text") msg += content.text; + } + msg = msg.trim(); + if (!msg) continue; + + // Clean up: remove wrapping quotes, backticks, trailing period + msg = msg.replace(/^[`"']|[`"']$/g, "").replace(/\.$/, ""); + + logger.debug("commit-msg-generator: generated", { model: model.id, msg }); + return msg; + } catch (err) { + logger.debug("commit-msg-generator: error", { + model: model.id, + error: err instanceof Error ? err.message : String(err), + }); + } + } + + return null; +} From 39ff2ab6e7977021f1841e279c06d84fdf017637 Mon Sep 17 00:00:00 2001 From: DeprecatedLuke <16418011+DeprecatedLuke@users.noreply.github.com> Date: Mon, 23 Feb 2026 19:15:59 +0000 Subject: [PATCH 5/7] feat(task): add task.eager setting for eager task delegation --- packages/coding-agent/CHANGELOG.md | 4 +- .../src/config/settings-schema.ts | 9 ++++ .../src/prompts/system/system-prompt.md | 13 ++++++ packages/coding-agent/src/sdk.ts | 3 ++ packages/coding-agent/src/system-prompt.ts | 4 ++ packages/coding-agent/src/task/index.ts | 21 +++++---- packages/coding-agent/src/task/worktree.ts | 43 +++++++++---------- 7 files changed, 65 insertions(+), 32 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 85ad5c984..998823a5a 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -9,15 +9,17 @@ ### 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 +- 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 cherry-picks for clean commit history - Added `task.isolation.commits` setting (`generic` or `ai`) for nested repo commit messages. `ai` mode uses a smol model to generate conventional commit messages from diffs - Nested non-submodule git repos are now discovered and handled during task isolation (changes captured and applied independently from parent repo) +- Added `task.eager` setting to encourage the agent to delegate work to subagents by default ### Fixed - Fixed nested repo changes being lost when tasks commit inside the isolation (baseline state is now committed before task runs, so delta correctly excludes it) - Fixed nested repo patches conflicting when multiple tasks contribute to the same repo (baseline untracked files no longer leak into patches) - Nested repo changes are now committed after patch application (previously left as untracked files) +- Failed tasks no longer create stale branches or capture garbage patches (gated on exit code) ## [13.2.0] - 2026-02-23 ### Breaking Changes diff --git a/packages/coding-agent/src/config/settings-schema.ts b/packages/coding-agent/src/config/settings-schema.ts index 71e485d71..d7e0ae506 100644 --- a/packages/coding-agent/src/config/settings-schema.ts +++ b/packages/coding-agent/src/config/settings-schema.ts @@ -577,6 +577,15 @@ export const SETTINGS_SCHEMA = { submenu: true, }, }, + "task.eager": { + type: "boolean", + default: false, + ui: { + tab: "tools", + label: "Eager task delegation", + description: "Encourage the agent to delegate work to subagents unless changes are trivial", + }, + }, "task.maxConcurrency": { type: "number", default: 32, diff --git a/packages/coding-agent/src/prompts/system/system-prompt.md b/packages/coding-agent/src/prompts/system/system-prompt.md index e733e2940..d62244f1b 100644 --- a/packages/coding-agent/src/prompts/system/system-prompt.md +++ b/packages/coding-agent/src/prompts/system/system-prompt.md @@ -270,6 +270,19 @@ Sequential work MUST be justified. If you cannot articulate why B depends on A, {{/has}} +{{#if eagerTasks}} + +You SHOULD delegate work to subagents by default. Working alone is the exception, not the rule. + +Use the Task tool unless the change is: +- A single-file edit under ~30 lines +- A direct answer or explanation with no code changes +- A command the user asked you to run yourself + +For everything else — multi-file changes, refactors, new features, test additions, investigations — break the work into tasks and delegate. Err on the side of delegating. You are an orchestrator first, a coder second. + +{{/if}} + Incomplete work means they start over — your effort wasted, their time lost. diff --git a/packages/coding-agent/src/sdk.ts b/packages/coding-agent/src/sdk.ts index 37412d1db..f30b47a25 100644 --- a/packages/coding-agent/src/sdk.ts +++ b/packages/coding-agent/src/sdk.ts @@ -1121,6 +1121,7 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} }); const repeatToolDescriptions = settings.get("repeatToolDescriptions"); + const eagerTasks = settings.get("task.eager"); const intentField = settings.get("tools.intentTracing") || $env.PI_INTENT_TRACING === "1" ? INTENT_FIELD : undefined; const rebuildSystemPrompt = async (toolNames: string[], tools: Map): Promise => { toolContextStore.setToolNames(toolNames); @@ -1136,6 +1137,7 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} skillsSettings: settings.getGroup("skills") as SkillsSettings, appendSystemPrompt: memoryInstructions, repeatToolDescriptions, + eagerTasks, intentField, }); @@ -1155,6 +1157,7 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} customPrompt: options.systemPrompt, appendSystemPrompt: memoryInstructions, repeatToolDescriptions, + eagerTasks, intentField, }); } diff --git a/packages/coding-agent/src/system-prompt.ts b/packages/coding-agent/src/system-prompt.ts index 2beaca69d..ca0a5c142 100644 --- a/packages/coding-agent/src/system-prompt.ts +++ b/packages/coding-agent/src/system-prompt.ts @@ -421,6 +421,8 @@ export interface BuildSystemPromptOptions { rules?: Array<{ name: string; description?: string; path: string; globs?: string[] }>; /** Intent field name injected into every tool schema. If set, explains the field in the prompt. */ intentField?: string; + /** Encourage the agent to delegate via tasks unless changes are trivial. */ + eagerTasks?: boolean; } /** Build the system prompt with tools, guidelines, and context */ @@ -442,6 +444,7 @@ export async function buildSystemPrompt(options: BuildSystemPromptOptions = {}): preloadedSkills: providedPreloadedSkills, rules, intentField, + eagerTasks = false, } = options; const resolvedCwd = cwd ?? getProjectDir(); const preloadedSkills = providedPreloadedSkills; @@ -614,5 +617,6 @@ export async function buildSystemPrompt(options: BuildSystemPromptOptions = {}): appendSystemPrompt: resolvedAppendPrompt ?? "", intentTracing: !!intentField, intentField: intentField ?? "", + eagerTasks, }); } diff --git a/packages/coding-agent/src/task/index.ts b/packages/coding-agent/src/task/index.ts index ec51ec1d3..b7a30d5e1 100644 --- a/packages/coding-agent/src/task/index.ts +++ b/packages/coding-agent/src/task/index.ts @@ -846,7 +846,7 @@ export class TaskTool implements AgentTool { preloadedSkills: task.preloadedSkills, promptTemplates, }); - if (mergeMode === "branch") { + if (mergeMode === "branch" && result.exitCode === 0) { const commitResult = await commitToBranch(isolationDir, baseline, task.id, task.description); return { ...result, @@ -854,14 +854,17 @@ export class TaskTool implements AgentTool { nestedPatches: commitResult?.nestedPatches, }; } - const delta = await captureDeltaPatch(isolationDir, baseline); - const patchPath = path.join(effectiveArtifactsDir, `${task.id}.patch`); - await Bun.write(patchPath, delta.rootPatch); - return { - ...result, - patchPath, - nestedPatches: delta.nestedPatches, - }; + if (result.exitCode === 0) { + const delta = await captureDeltaPatch(isolationDir, baseline); + const patchPath = path.join(effectiveArtifactsDir, `${task.id}.patch`); + await Bun.write(patchPath, delta.rootPatch); + return { + ...result, + patchPath, + nestedPatches: delta.nestedPatches, + }; + } + return result; } catch (err) { const message = err instanceof Error ? err.message : String(err); return { diff --git a/packages/coding-agent/src/task/worktree.ts b/packages/coding-agent/src/task/worktree.ts index efb6256e7..45a69d027 100644 --- a/packages/coding-agent/src/task/worktree.ts +++ b/packages/coding-agent/src/task/worktree.ts @@ -2,16 +2,10 @@ import type { Dirent } from "node:fs"; import * as fs from "node:fs/promises"; import * as os from "node:os"; import path from "node:path"; -import { isEnoent, Snowflake } from "@oh-my-pi/pi-utils"; +import { isEnoent, logger, Snowflake } from "@oh-my-pi/pi-utils"; import { getWorktreeDir } from "@oh-my-pi/pi-utils/dirs"; import { $ } from "bun"; -async function gitAuthorFlags(cwd: string): Promise { - const name = (await $`git config user.name`.cwd(cwd).quiet().nothrow().text()).trim() || "omp"; - const email = (await $`git config user.email`.cwd(cwd).quiet().nothrow().text()).trim() || "omp@task"; - return [`-c`, `user.name=${name}`, `-c`, `user.email=${email}`]; -} - /** Baseline state for a single git repository. */ export interface RepoBaseline { repoRoot: string; @@ -191,8 +185,7 @@ export async function applyBaseline(worktreeDir: string, baseline: WorktreeBasel const hasChanges = (await $`git status --porcelain`.cwd(nestedDir).quiet().nothrow().text()).trim(); if (hasChanges) { await $`git add -A`.cwd(nestedDir).quiet(); - const flags = await gitAuthorFlags(nestedDir); - await $`git ${flags} commit -m omp-baseline --allow-empty`.cwd(nestedDir).quiet(); + await $`git commit -m omp-baseline --allow-empty`.cwd(nestedDir).quiet(); // Update baseline to reflect the committed state — prevents double-apply // in captureRepoDeltaPatch's temp-index path entry.baseline.headCommit = (await $`git rev-parse HEAD`.cwd(nestedDir).quiet().text()).trim(); @@ -355,8 +348,7 @@ export async function applyNestedPatches( if (hasChanges) { const msg = (await commitMessage?.(combinedDiff)) ?? "changes from isolated task(s)"; await $`git add -A`.cwd(nestedDir).quiet(); - const flags = await gitAuthorFlags(nestedDir); - await $`git ${flags} commit -m ${msg}`.cwd(nestedDir).quiet(); + await $`git commit -m ${msg}`.cwd(nestedDir).quiet(); } } } @@ -467,13 +459,23 @@ export async function commitToBranch( const patchPath = path.join(os.tmpdir(), `omp-branch-patch-${Snowflake.next()}.patch`); try { await Bun.write(patchPath, rootPatch); - await $`git apply --binary ${patchPath}`.cwd(tmpDir).quiet(); + const applyResult = await $`git apply --binary ${patchPath}`.cwd(tmpDir).quiet().nothrow(); + if (applyResult.exitCode !== 0) { + const stderr = applyResult.stderr.toString().slice(0, 2000); + logger.error("commitToBranch: git apply failed", { + taskId, + exitCode: applyResult.exitCode, + stderr, + patchSize: rootPatch.length, + patchHead: rootPatch.slice(0, 500), + }); + throw new Error(`git apply failed for task ${taskId}: ${stderr}`); + } } finally { await fs.rm(patchPath, { force: true }); } await $`git add -A`.cwd(tmpDir).quiet(); - const flags = await gitAuthorFlags(tmpDir); - await $`git ${flags} commit -m ${commitMessage}`.cwd(tmpDir).quiet(); + await $`git 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 }); @@ -490,8 +492,8 @@ export interface MergeBranchResult { } /** - * Merge task branches sequentially into the working tree. - * Each branch gets a --no-ff merge commit preserving the task identity. + * 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. */ export async function mergeTaskBranches( @@ -501,14 +503,11 @@ export async function mergeTaskBranches( const merged: string[] = []; const failed: string[] = []; - for (const { branchName, taskId, description } of branches) { - const mergeMessage = description || taskId; - - const result = await $`git merge --no-ff -m ${mergeMessage} ${branchName}`.cwd(repoRoot).quiet().nothrow(); + for (const { branchName } of branches) { + const result = await $`git cherry-pick ${branchName}`.cwd(repoRoot).quiet().nothrow(); if (result.exitCode !== 0) { - // Abort the failed merge to restore clean state - await $`git merge --abort`.cwd(repoRoot).quiet().nothrow(); + await $`git cherry-pick --abort`.cwd(repoRoot).quiet().nothrow(); const stderr = result.stderr.toString().trim(); failed.push(branchName); return { From 749937975685a5f43c7da4f73768e6b6f76ff446 Mon Sep 17 00:00:00 2001 From: DeprecatedLuke <16418011+DeprecatedLuke@users.noreply.github.com> Date: Mon, 23 Feb 2026 20:03:55 +0000 Subject: [PATCH 6/7] fix(task): make merge failures non-fatal, preserve agent output --- packages/coding-agent/src/task/index.ts | 69 ++++++++++++++----- packages/coding-agent/src/task/render.ts | 30 +++++--- packages/coding-agent/src/task/worktree.ts | 6 +- .../src/utils/commit-message-generator.ts | 27 +++++++- 4 files changed, 105 insertions(+), 27 deletions(-) diff --git a/packages/coding-agent/src/task/index.ts b/packages/coding-agent/src/task/index.ts index b7a30d5e1..73e272913 100644 --- a/packages/coding-agent/src/task/index.ts +++ b/packages/coding-agent/src/task/index.ts @@ -847,22 +847,53 @@ export class TaskTool implements AgentTool { promptTemplates, }); if (mergeMode === "branch" && result.exitCode === 0) { - const commitResult = await commitToBranch(isolationDir, baseline, task.id, task.description); - return { - ...result, - branchName: commitResult?.branchName, - nestedPatches: commitResult?.nestedPatches, - }; + try { + const commitMsg = + commitStyle === "ai" && this.session.modelRegistry + ? async (diff: string) => { + const smolModel = this.session.settings.getModelRole("smol"); + return generateCommitMessage( + diff, + this.session.modelRegistry!, + smolModel, + this.session.getSessionId?.() ?? undefined, + ); + } + : undefined; + const commitResult = await commitToBranch( + isolationDir, + baseline, + task.id, + task.description, + commitMsg, + ); + return { + ...result, + branchName: commitResult?.branchName, + nestedPatches: commitResult?.nestedPatches, + }; + } catch (mergeErr) { + // Agent succeeded but branch commit failed — clean up stale branch + const branchName = `omp/task/${task.id}`; + await $`git branch -D ${branchName}`.cwd(baseline.root.repoRoot).quiet().nothrow(); + const msg = mergeErr instanceof Error ? mergeErr.message : String(mergeErr); + return { ...result, error: `Merge failed: ${msg}` }; + } } if (result.exitCode === 0) { - const delta = await captureDeltaPatch(isolationDir, baseline); - const patchPath = path.join(effectiveArtifactsDir, `${task.id}.patch`); - await Bun.write(patchPath, delta.rootPatch); - return { - ...result, - patchPath, - nestedPatches: delta.nestedPatches, - }; + try { + const delta = await captureDeltaPatch(isolationDir, baseline); + const patchPath = path.join(effectiveArtifactsDir, `${task.id}.patch`); + await Bun.write(patchPath, delta.rootPatch); + return { + ...result, + patchPath, + nestedPatches: delta.nestedPatches, + }; + } catch (patchErr) { + const msg = patchErr instanceof Error ? patchErr.message : String(patchErr); + return { ...result, error: `Patch capture failed: ${msg}` }; + } } return result; } catch (err) { @@ -1070,12 +1101,18 @@ export class TaskTool implements AgentTool { } // Build final output - match plugin format - const successCount = results.filter(r => r.exitCode === 0).length; + const successCount = results.filter(r => r.exitCode === 0 && !r.error).length; const cancelledCount = results.filter(r => r.aborted).length; const totalDuration = Date.now() - startTime; const summaries = results.map(r => { - const status = r.aborted ? "cancelled" : r.exitCode === 0 ? "completed" : `failed (exit ${r.exitCode})`; + const status = r.aborted + ? "cancelled" + : r.exitCode === 0 && r.error + ? "merge failed" + : r.exitCode === 0 + ? "completed" + : `failed (exit ${r.exitCode})`; const output = r.output.trim() || r.stderr.trim() || "(no output)"; const outputCharCount = r.outputMeta?.charCount ?? output.length; const fullOutputThreshold = 5000; diff --git a/packages/coding-agent/src/task/render.ts b/packages/coding-agent/src/task/render.ts index 39f78f25c..cfefaea9b 100644 --- a/packages/coding-agent/src/task/render.ts +++ b/packages/coding-agent/src/task/render.ts @@ -728,7 +728,8 @@ function renderAgentResult(result: SingleResult, isLast: boolean, expanded: bool result.output, ); const aborted = result.aborted ?? false; - const success = !aborted && result.exitCode === 0; + const mergeFailed = !aborted && result.exitCode === 0 && !!result.error; + const success = !aborted && result.exitCode === 0 && !result.error; const needsWarning = Boolean(missingCompleteWarning) && success; const icon = aborted ? theme.status.aborted @@ -737,8 +738,16 @@ function renderAgentResult(result: SingleResult, isLast: boolean, expanded: bool : success ? theme.status.success : theme.status.error; - const iconColor = needsWarning ? "warning" : success ? "success" : "error"; - const statusText = aborted ? "aborted" : needsWarning ? "warning" : success ? "done" : "failed"; + const iconColor = needsWarning ? "warning" : success ? "success" : mergeFailed ? "warning" : "error"; + const statusText = aborted + ? "aborted" + : needsWarning + ? "warning" + : success + ? "done" + : mergeFailed + ? "merge failed" + : "failed"; // Main status line: id: description [status] · stats · ⟨agent⟩ const description = result.description?.trim(); @@ -852,8 +861,8 @@ function renderAgentResult(result: SingleResult, isLast: boolean, expanded: bool } // Error message - if (result.error && !success) { - lines.push(`${continuePrefix}${theme.fg("error", truncateToWidth(result.error, 70))}`); + if (result.error && (!success || mergeFailed)) { + lines.push(`${continuePrefix}${theme.fg(mergeFailed ? "warning" : "error", truncateToWidth(result.error, 70))}`); } return lines; @@ -904,15 +913,20 @@ export function renderResult( }); const abortedCount = details.results.filter(r => r.aborted).length; - const successCount = details.results.filter(r => !r.aborted && r.exitCode === 0).length; - const failCount = details.results.length - successCount - abortedCount; + const mergeFailedCount = details.results.filter(r => !r.aborted && r.exitCode === 0 && r.error).length; + const successCount = details.results.filter(r => !r.aborted && r.exitCode === 0 && !r.error).length; + const failCount = details.results.length - successCount - mergeFailedCount - abortedCount; let summary = `${theme.fg("dim", "Total:")} `; if (abortedCount > 0) { summary += theme.fg("error", `${abortedCount} aborted`); - if (successCount > 0 || failCount > 0) summary += theme.sep.dot; + if (successCount > 0 || mergeFailedCount > 0 || failCount > 0) summary += theme.sep.dot; } if (successCount > 0) { summary += theme.fg("success", `${successCount} succeeded`); + if (mergeFailedCount > 0 || failCount > 0) summary += theme.sep.dot; + } + if (mergeFailedCount > 0) { + summary += theme.fg("warning", `${mergeFailedCount} merge failed`); if (failCount > 0) summary += theme.sep.dot; } if (failCount > 0) { diff --git a/packages/coding-agent/src/task/worktree.ts b/packages/coding-agent/src/task/worktree.ts index 45a69d027..59e24ec58 100644 --- a/packages/coding-agent/src/task/worktree.ts +++ b/packages/coding-agent/src/task/worktree.ts @@ -442,13 +442,14 @@ export async function commitToBranch( baseline: WorktreeBaseline, taskId: string, description: string | undefined, + commitMessage?: (diff: string) => Promise, ): 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; + const fallbackMessage = description || taskId; // Only create a branch if the root repo has changes if (rootPatch.trim()) { @@ -475,7 +476,8 @@ export async function commitToBranch( await fs.rm(patchPath, { force: true }); } await $`git add -A`.cwd(tmpDir).quiet(); - await $`git commit -m ${commitMessage}`.cwd(tmpDir).quiet(); + const msg = (commitMessage && (await commitMessage(rootPatch))) || fallbackMessage; + await $`git commit -m ${msg}`.cwd(tmpDir).quiet(); } finally { await $`git worktree remove -f ${tmpDir}`.cwd(repoRoot).quiet().nothrow(); await fs.rm(tmpDir, { recursive: true, force: true }); diff --git a/packages/coding-agent/src/utils/commit-message-generator.ts b/packages/coding-agent/src/utils/commit-message-generator.ts index 71737c849..5691bcf26 100644 --- a/packages/coding-agent/src/utils/commit-message-generator.ts +++ b/packages/coding-agent/src/utils/commit-message-generator.ts @@ -14,6 +14,25 @@ import commitSystemPrompt from "../prompts/system/commit-message-system.md" with const COMMIT_SYSTEM_PROMPT = renderPromptTemplate(commitSystemPrompt); const MAX_DIFF_CHARS = 4000; +/** Paths that should be excluded from commit message generation diffs. */ +const NOISE_PATH_PREFIXES = ["node_modules/", ".yarn/", ".pnp.", "dist/", "build/", ".next/", "coverage/"]; + +/** Strip diff hunks for noisy paths that drown out real changes. */ +function filterDiffNoise(diff: string): string { + const lines = diff.split("\n"); + const filtered: string[] = []; + let skip = false; + for (const line of lines) { + if (line.startsWith("diff --git ")) { + // Extract b/ path from "diff --git a/... b/..." + const bPath = line.split(" b/")[1]; + skip = bPath != null && NOISE_PATH_PREFIXES.some(p => bPath.startsWith(p)); + } + if (!skip) filtered.push(line); + } + return filtered.join("\n"); +} + function getSmolModelCandidates(registry: ModelRegistry, savedSmolModel?: string): Model[] { const availableModels = registry.getAvailable(); if (availableModels.length === 0) return []; @@ -62,7 +81,13 @@ export async function generateCommitMessage( return null; } - const truncatedDiff = diff.length > MAX_DIFF_CHARS ? `${diff.slice(0, MAX_DIFF_CHARS)}\n… (truncated)` : diff; + const cleanDiff = filterDiffNoise(diff); + const truncatedDiff = + cleanDiff.length > MAX_DIFF_CHARS ? `${cleanDiff.slice(0, MAX_DIFF_CHARS)}\n… (truncated)` : cleanDiff; + if (!truncatedDiff.trim()) { + logger.debug("commit-msg-generator: diff is empty after noise filtering"); + return null; + } const userMessage = `\n${truncatedDiff}\n`; for (const model of candidates) { From 745e38aebfece02f8c1c179f98f70eb885de1129 Mon Sep 17 00:00:00 2001 From: DeprecatedLuke <16418011+DeprecatedLuke@users.noreply.github.com> Date: Mon, 23 Feb 2026 20:25:30 +0000 Subject: [PATCH 7/7] fix(task): filter lock files from commit message diff input --- packages/coding-agent/CHANGELOG.md | 5 ++++- packages/coding-agent/DEVELOPMENT.md | 8 +++++--- packages/coding-agent/src/task/index.ts | 1 - .../coding-agent/src/utils/commit-message-generator.ts | 9 ++++----- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 998823a5a..dc329ae3e 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -10,7 +10,7 @@ - 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 cherry-picks for clean commit history -- Added `task.isolation.commits` setting (`generic` or `ai`) for nested repo commit messages. `ai` mode uses a smol model to generate conventional commit messages from diffs +- Added `task.isolation.commits` setting (`generic` or `ai`) for commit messages on isolated task branches and nested repos. `ai` mode uses a smol model to generate conventional commit messages from diffs - Nested non-submodule git repos are now discovered and handled during task isolation (changes captured and applied independently from parent repo) - Added `task.eager` setting to encourage the agent to delegate work to subagents by default @@ -20,6 +20,9 @@ - Fixed nested repo patches conflicting when multiple tasks contribute to the same repo (baseline untracked files no longer leak into patches) - Nested repo changes are now committed after patch application (previously left as untracked files) - Failed tasks no longer create stale branches or capture garbage patches (gated on exit code) +- Merge failures (e.g. conflicting patches) are now non-fatal — agent output is preserved with `merge failed` status instead of `failed` +- Stale branches are cleaned up when `commitToBranch` fails +- Commit message generator filters lock files from diffs before AI summarization ## [13.2.0] - 2026-02-23 ### Breaking Changes diff --git a/packages/coding-agent/DEVELOPMENT.md b/packages/coding-agent/DEVELOPMENT.md index cff1843f5..a9384b85d 100644 --- a/packages/coding-agent/DEVELOPMENT.md +++ b/packages/coding-agent/DEVELOPMENT.md @@ -907,11 +907,13 @@ Despite the name `runSubprocess`, `packages/coding-agent/src/task/executor.ts` c 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. + - **worktree**: `ensureWorktree(...)`, `applyBaseline(...)`, `captureDeltaPatch(...)`, `cleanupWorktree(...)`. Nested non-submodule git repos are discovered and handled independently. + - **fuse-overlay**: `ensureFuseOverlay(...)` (mounts a copy-on-write overlay via `fuse-overlayfs`), `captureDeltaPatch(...)`, `cleanupFuseOverlay(...)`. No baseline apply needed since the overlay reflects the full working tree. Fails outright if mount fails. - 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. + - **branch**: each task commits to a temp branch (`omp/task/`) via `commitToBranch(...)`, then `mergeTaskBranches(...)` cherry-picks them sequentially onto HEAD. If `git apply` fails inside `commitToBranch`, the error is non-fatal — the agent result is preserved with a `merge failed` status. +- The `task.isolation.commits` setting (`generic` or `ai`) controls commit messages for branch commits and nested repo patches. `ai` mode uses a smol model to generate conventional commit messages from diffs. +- Nested repo patches are applied via `applyNestedPatches(...)` after the parent merge, grouped by repo with one commit per repo. - 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/task/index.ts b/packages/coding-agent/src/task/index.ts index 73e272913..824e4fa62 100644 --- a/packages/coding-agent/src/task/index.ts +++ b/packages/coding-agent/src/task/index.ts @@ -806,7 +806,6 @@ export class TaskTool implements AgentTool { if (isolationMode === "fuse-overlay") { isolationDir = await ensureFuseOverlay(repoRoot, task.id); - // Overlay already reflects the full working tree state — no baseline apply needed } else { isolationDir = await ensureWorktree(repoRoot, task.id); await applyBaseline(isolationDir, baseline); diff --git a/packages/coding-agent/src/utils/commit-message-generator.ts b/packages/coding-agent/src/utils/commit-message-generator.ts index 5691bcf26..726ceca20 100644 --- a/packages/coding-agent/src/utils/commit-message-generator.ts +++ b/packages/coding-agent/src/utils/commit-message-generator.ts @@ -14,19 +14,18 @@ import commitSystemPrompt from "../prompts/system/commit-message-system.md" with const COMMIT_SYSTEM_PROMPT = renderPromptTemplate(commitSystemPrompt); const MAX_DIFF_CHARS = 4000; -/** Paths that should be excluded from commit message generation diffs. */ -const NOISE_PATH_PREFIXES = ["node_modules/", ".yarn/", ".pnp.", "dist/", "build/", ".next/", "coverage/"]; +/** File patterns that should be excluded from commit message generation diffs. */ +const NOISE_SUFFIXES = [".lock", ".lockb", "-lock.json", "-lock.yaml"]; -/** Strip diff hunks for noisy paths that drown out real changes. */ +/** Strip diff hunks for noisy files that drown out real changes. */ function filterDiffNoise(diff: string): string { const lines = diff.split("\n"); const filtered: string[] = []; let skip = false; for (const line of lines) { if (line.startsWith("diff --git ")) { - // Extract b/ path from "diff --git a/... b/..." const bPath = line.split(" b/")[1]; - skip = bPath != null && NOISE_PATH_PREFIXES.some(p => bPath.startsWith(p)); + skip = bPath != null && NOISE_SUFFIXES.some(s => bPath.endsWith(s)); } if (!skip) filtered.push(line); }