diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index b3b97a8ab..a0f3a1155 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,9 +2,28 @@ ## [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 `PERPLEXITY_COOKIES` env var for Perplexity web search via session cookies extracted from desktop app +- 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 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 + +### 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) +- 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.1] - 2026-02-24 @@ -15,7 +34,6 @@ ### Changed - Extracted non-interactive environment config from `bash-interactive.ts` into shared `non-interactive-env.ts` module, applied consistently to all bash execution paths - ## [13.2.0] - 2026-02-23 ### Breaking Changes diff --git a/packages/coding-agent/DEVELOPMENT.md b/packages/coding-agent/DEVELOPMENT.md index e02010b39..796a17f7c 100644 --- a/packages/coding-agent/DEVELOPMENT.md +++ b/packages/coding-agent/DEVELOPMENT.md @@ -906,7 +906,14 @@ 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(...)`. 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(...)` 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/config/settings-schema.ts b/packages/coding-agent/src/config/settings-schema.ts index 1177433f1..d7e0ae506 100644 --- a/packages/coding-agent/src/config/settings-schema.ts +++ b/packages/coding-agent/src/config/settings-schema.ts @@ -544,14 +544,46 @@ export const SETTINGS_SCHEMA = { // ───────────────────────────────────────────────────────────────────────── // Task tool settings // ───────────────────────────────────────────────────────────────────────── - "task.isolation.enabled": { + "task.isolation.mode": { + type: "enum", + values: ["none", "worktree", "fuse-overlay"] as const, + default: "none", + ui: { + tab: "tools", + label: "Task isolation", + description: "Isolation mode for subagents (none, git worktree, or fuse-overlay)", + 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.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.eager": { type: "boolean", default: false, ui: { tab: "tools", - label: "Task isolation", - description: "Run subagents in isolated git worktrees", - submenu: true, + label: "Eager task delegation", + description: "Encourage the agent to delegate work to subagents unless changes are trivial", }, }, "task.maxConcurrency": { diff --git a/packages/coding-agent/src/config/settings.ts b/packages/coding-agent/src/config/settings.ts index 61acae85c..0615d87bd 100644 --- a/packages/coding-agent/src/config/settings.ts +++ b/packages/coding-agent/src/config/settings.ts @@ -553,6 +553,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/modes/components/settings-defs.ts b/packages/coding-agent/src/modes/components/settings-defs.ts index f129a9b65..5bfc54e03 100644 --- a/packages/coding-agent/src/modes/components/settings-defs.ts +++ b/packages/coding-agent/src/modes/components/settings-defs.ts @@ -93,6 +93,22 @@ 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" }, + ], + // 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/prompts/system/system-prompt.md b/packages/coding-agent/src/prompts/system/system-prompt.md index a637367db..dc8baefc1 100644 --- a/packages/coding-agent/src/prompts/system/system-prompt.md +++ b/packages/coding-agent/src/prompts/system/system-prompt.md @@ -159,6 +159,19 @@ Semantic questions **MUST** be answered with semantic tools. - What is this thing? → `lsp hover` {{/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}} + {{#has tools "ssh"}} ### SSH: match commands to host shell 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/prompts/tools/task.md b/packages/coding-agent/src/prompts/tools/task.md index ec515a13b..d703725d3 100644 --- a/packages/coding-agent/src/prompts/tools/task.md +++ b/packages/coding-agent/src/prompts/tools/task.md @@ -16,7 +16,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/sdk.ts b/packages/coding-agent/src/sdk.ts index d83439ddc..85c8c38a4 100644 --- a/packages/coding-agent/src/sdk.ts +++ b/packages/coding-agent/src/sdk.ts @@ -1120,6 +1120,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); @@ -1135,6 +1136,7 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} skillsSettings: settings.getGroup("skills") as SkillsSettings, appendSystemPrompt: memoryInstructions, repeatToolDescriptions, + eagerTasks, intentField, }); @@ -1154,6 +1156,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 edb61fe32..6c3f8c611 100644 --- a/packages/coding-agent/src/system-prompt.ts +++ b/packages/coding-agent/src/system-prompt.ts @@ -358,6 +358,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 */ @@ -379,6 +381,7 @@ export async function buildSystemPrompt(options: BuildSystemPromptOptions = {}): preloadedSkills: providedPreloadedSkills, rules, intentField, + eagerTasks = false, } = options; const resolvedCwd = cwd ?? getProjectDir(); const preloadedSkills = providedPreloadedSkills; @@ -535,6 +538,7 @@ export async function buildSystemPrompt(options: BuildSystemPromptOptions = {}): cwd: resolvedCwd, intentTracing: !!intentField, intentField: intentField ?? "", + eagerTasks, }; return renderPromptTemplate(resolvedCustomPrompt ? customSystemPromptTemplate : systemPromptTemplate, data); } diff --git a/packages/coding-agent/src/task/index.ts b/packages/coding-agent/src/task/index.ts index 88c073b21..27bc4ae6d 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"; @@ -47,11 +48,17 @@ import { } from "./types"; import { applyBaseline, + applyNestedPatches, captureBaseline, captureDeltaPatch, + cleanupFuseOverlay, + cleanupTaskBranches, cleanupWorktree, + commitToBranch, + ensureFuseOverlay, ensureWorktree, getRepoRoot, + mergeTaskBranches, type WorktreeBaseline, } from "./worktree"; @@ -145,11 +152,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 +175,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 +429,20 @@ 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 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; - 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 +798,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); + const taskBaseline = structuredClone(baseline); + + if (isolationMode === "fuse-overlay") { + isolationDir = await ensureFuseOverlay(repoRoot, task.id); + } else { + isolationDir = await ensureWorktree(repoRoot, task.id); + await applyBaseline(isolationDir, taskBaseline); + } + const result = await runSubprocess({ cwd: this.session.cwd, - worktree: worktreeDir, + worktree: isolationDir, agent, task: task.task, description: task.description, @@ -830,13 +846,56 @@ export class TaskTool implements AgentTool { preloadedSkills: task.preloadedSkills, promptTemplates, }); - const patch = await captureDeltaPatch(worktreeDir, baseline); - const patchPath = path.join(effectiveArtifactsDir, `${task.id}.patch`); - await Bun.write(patchPath, patch); - return { - ...result, - patchPath, - }; + if (mergeMode === "branch" && result.exitCode === 0) { + 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, + taskBaseline, + 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(repoRoot).quiet().nothrow(); + const msg = mergeErr instanceof Error ? mergeErr.message : String(mergeErr); + return { ...result, error: `Merge failed: ${msg}` }; + } + } + if (result.exitCode === 0) { + try { + const delta = await captureDeltaPatch(isolationDir, taskBaseline); + 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) { const message = err instanceof Error ? err.message : String(err); return { @@ -856,8 +915,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); + } } } }; @@ -917,74 +980,152 @@ 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; + let mergedBranchesForNestedPatches: Set | 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); + mergedBranchesForNestedPatches = new Set(mergeResult.merged); + changesApplied = mergeResult.failed.length === 0; + + if (changesApplied) { + mergeSummary = `\n\nMerged ${mergeResult.merged.length} branch${mergeResult.merged.length === 1 ? "" : "es"}: ${mergeResult.merged.join(", ")}`; } else { - const 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 && (mergeMode === "branch" || changesApplied !== false)) { + const allNestedPatches = results + .filter(r => { + if (!r.nestedPatches || r.nestedPatches.length === 0 || r.exitCode !== 0 || r.aborted) { + return false; + } + if (mergeMode !== "branch") { + return true; + } + if (!r.branchName || !mergedBranchesForNestedPatches) { + return false; + } + return mergedBranchesForNestedPatches.has(r.branchName); + }) + .flatMap(r => r.nestedPatches!); + if (allNestedPatches.length > 0) { + 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; + await applyNestedPatches(repoRoot, allNestedPatches, commitMsg); + } catch { + // Nested patch failures are non-fatal to the parent merge + mergeSummary += + "\n\nSome nested repository patches failed to apply."; + } } } // 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; @@ -1021,12 +1162,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..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(); @@ -847,11 +856,13 @@ 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 - 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; @@ -902,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/types.ts b/packages/coding-agent/src/task/types.ts index fbe6d43cd..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"; @@ -77,7 +78,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.", }), ), }); @@ -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 f49af82f5..93cc09d3a 100644 --- a/packages/coding-agent/src/task/worktree.ts +++ b/packages/coding-agent/src/task/worktree.ts @@ -1,16 +1,26 @@ +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 { getWorktreeDir, isEnoent, Snowflake } from "@oh-my-pi/pi-utils"; +import { getWorktreeDir, isEnoent, logger, Snowflake } from "@oh-my-pi/pi-utils"; import { $ } from "bun"; -export interface WorktreeBaseline { +/** Baseline state for a single git repository. */ +export interface RepoBaseline { repoRoot: string; + headCommit: 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, "-")}--`; } @@ -38,7 +48,56 @@ 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 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(); @@ -46,13 +105,18 @@ export async function captureBaseline(repoRoot: string): Promise line.trim()) .filter(line => line.length > 0); + return { repoRoot, headCommit, 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 { @@ -80,13 +144,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 }); @@ -98,6 +162,39 @@ 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 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, entry.relativePath); + try { + await fs.cp(sourceDir, nestedDir, { recursive: true }); + } catch (err) { + if (isEnoent(err)) continue; + throw err; + } + // 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(); + 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(); + entry.baseline.staged = ""; + entry.baseline.unstaged = ""; + entry.baseline.untracked = []; + } + } +} + async function applyPatchToIndex(cwd: string, patch: string, indexFile: string): Promise { if (!patch.trim()) return; const tempPath = await writeTempPatchFile(patch); @@ -121,31 +218,62 @@ 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 { + // 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(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 ${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(); - 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")}`; @@ -154,6 +282,76 @@ export async function captureDeltaPatch(worktreeDir: string, baseline: WorktreeB } } +export interface NestedRepoPatch { + relativePath: string; + patch: string; +} + +export interface DeltaPatchResult { + rootPatch: string; + nestedPatches: NestedRepoPatch[]; +} + +export async function captureDeltaPatch(isolationDir: string, baseline: WorktreeBaseline): Promise { + const rootPatch = await captureRepoDeltaPatch(isolationDir, baseline.root); + 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; + } + const patch = await captureRepoDeltaPatch(nestedDir, nb); + if (patch.trim()) nestedPatches.push({ relativePath, patch }); + } + + return { rootPatch, nestedPatches }; +} + +/** + * 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; + } + + 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(); + await $`git commit -m ${msg}`.cwd(nestedDir).quiet(); + } + } +} + export async function cleanupWorktree(dir: string): Promise { try { const commonDirRaw = await $`git rev-parse --git-common-dir`.cwd(dir).quiet().nothrow().text(); @@ -167,3 +365,168 @@ 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 }); + } +} + +// ═══════════════════════════════════════════════════════════════════════════ +// Branch-mode isolation +// ═══════════════════════════════════════════════════════════════════════════ + +export interface CommitToBranchResult { + branchName?: string; + nestedPatches: NestedRepoPatch[]; +} + +/** + * Commit task-only changes to a new branch. + * Only root repo changes go on the branch. Nested repo patches are returned + * separately since the parent git can't track files inside gitlinks. + */ +export async function commitToBranch( + isolationDir: string, + 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 fallbackMessage = description || taskId; + + // 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); + 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 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 }); + } + } + + return { branchName: rootPatch.trim() ? branchName : undefined, nestedPatches }; +} + +export interface MergeBranchResult { + merged: string[]; + failed: string[]; + conflict?: string; +} + +/** + * 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( + repoRoot: string, + branches: Array<{ branchName: string; taskId: string; description?: string }>, +): Promise { + const merged: string[] = []; + const failed: string[] = []; + + for (const { branchName } of branches) { + const result = await $`git cherry-pick ${branchName}`.cwd(repoRoot).quiet().nothrow(); + + if (result.exitCode !== 0) { + await $`git cherry-pick --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(); + } +} 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..726ceca20 --- /dev/null +++ b/packages/coding-agent/src/utils/commit-message-generator.ts @@ -0,0 +1,132 @@ +/** + * 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; + +/** 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 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 ")) { + const bPath = line.split(" b/")[1]; + skip = bPath != null && NOISE_SUFFIXES.some(s => bPath.endsWith(s)); + } + 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 []; + + 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 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) { + 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; +} diff --git a/packages/tui/src/terminal-capabilities.ts b/packages/tui/src/terminal-capabilities.ts index ffe84214d..ec7613c38 100644 --- a/packages/tui/src/terminal-capabilities.ts +++ b/packages/tui/src/terminal-capabilities.ts @@ -234,9 +234,12 @@ function calculateImageFit( const fittedWidthPx = imageDimensions.widthPx * scale; const fittedHeightPx = imageDimensions.heightPx * scale; + const columns = Math.max(1, Math.floor(fittedWidthPx / cellDims.widthPx)); + const rows = Math.max(1, Math.ceil(fittedHeightPx / cellDims.heightPx)); + return { - columns: Math.max(1, Math.floor(fittedWidthPx / cellDims.widthPx)), - rows: Math.max(1, Math.ceil(fittedHeightPx / cellDims.heightPx)), + columns: maxColumns !== undefined ? Math.min(columns, maxColumns) : columns, + rows: maxRows !== undefined ? Math.min(rows, maxRows) : rows, }; }