From 54df619891460f0c512587cb626b947eb5f75817 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=82CHES?= Date: Fri, 13 Mar 2026 16:10:55 -0600 Subject: [PATCH] feat: task isolation for subagent filesystem safety (#254) * feat: add task isolation for subagent filesystem safety Subagents can run in isolated git worktrees (or FUSE overlays on Linux) so concurrent tasks don't stomp on each other's files. Changes are captured as unified diffs and merged back via git apply. - New isolation.ts module with worktree and FUSE overlay backends - TaskIsolationSettings in settings-manager (mode + merge strategy) - isolated parameter on the subagent tool schema - Baseline capture/apply mirrors the parent repo's dirty state - Process exit handler for best-effort cleanup of stale worktrees Co-Authored-By: Claude Opus 4.6 (1M context) * fix: correct delta capture to exclude parent baseline state The worktree backend now commits a baseline snapshot after applying the parent's dirty state, so captureDeltaPatch diffs only the subagent's actual changes against the post-baseline HEAD (not the original HEAD). The FUSE overlay backend tracks the parent's dirty file set at mount time and filters the upper dir during delta capture to exclude inherited dirty files. Also removes dead code: findGitRoot (unused), readIsolationMergeStrategy (exported but never called). Co-Authored-By: Claude Opus 4.6 (1M context) --------- Co-authored-by: Claude Opus 4.6 (1M context) --- .../src/core/settings-manager.ts | 14 + packages/pi-coding-agent/src/index.ts | 1 + src/resources/extensions/subagent/index.ts | 93 +++- .../extensions/subagent/isolation.ts | 498 ++++++++++++++++++ 4 files changed, 585 insertions(+), 21 deletions(-) create mode 100644 src/resources/extensions/subagent/isolation.ts diff --git a/packages/pi-coding-agent/src/core/settings-manager.ts b/packages/pi-coding-agent/src/core/settings-manager.ts index 09f23c933..5d4592b4b 100644 --- a/packages/pi-coding-agent/src/core/settings-manager.ts +++ b/packages/pi-coding-agent/src/core/settings-manager.ts @@ -54,6 +54,11 @@ export interface AsyncSettings { maxJobs?: number; // default: 100 } +export interface TaskIsolationSettings { + mode?: "none" | "worktree" | "fuse-overlay"; // default: "none" + merge?: "patch" | "branch"; // default: "patch" +} + export type TransportSetting = Transport; /** @@ -106,6 +111,7 @@ export interface Settings { markdown?: MarkdownSettings; async?: AsyncSettings; bashInterceptor?: BashInterceptorSettings; + taskIsolation?: TaskIsolationSettings; } /** Deep merge settings: project/overrides take precedence, nested objects merge recursively */ @@ -968,4 +974,12 @@ export class SettingsManager { getAsyncMaxJobs(): number { return this.settings.async?.maxJobs ?? 100; } + + getTaskIsolationMode(): "none" | "worktree" | "fuse-overlay" { + return this.settings.taskIsolation?.mode ?? "none"; + } + + getTaskIsolationMerge(): "patch" | "branch" { + return this.settings.taskIsolation?.merge ?? "patch"; + } } diff --git a/packages/pi-coding-agent/src/index.ts b/packages/pi-coding-agent/src/index.ts index 40bc8dd53..ab78cefa6 100644 --- a/packages/pi-coding-agent/src/index.ts +++ b/packages/pi-coding-agent/src/index.ts @@ -208,6 +208,7 @@ export { type PackageSource, type RetrySettings, SettingsManager, + type TaskIsolationSettings, } from "./core/settings-manager.js"; // Skills export { diff --git a/src/resources/extensions/subagent/index.ts b/src/resources/extensions/subagent/index.ts index c849744f5..f598327e3 100644 --- a/src/resources/extensions/subagent/index.ts +++ b/src/resources/extensions/subagent/index.ts @@ -13,6 +13,7 @@ */ import { spawn } from "node:child_process"; +import * as crypto from "node:crypto"; import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; @@ -23,6 +24,14 @@ import { type ExtensionAPI, getMarkdownTheme } from "@gsd/pi-coding-agent"; import { Container, Markdown, Spacer, Text } from "@gsd/pi-tui"; import { Type } from "@sinclair/typebox"; import { type AgentConfig, type AgentScope, discoverAgents } from "./agents.js"; +import { + type IsolationEnvironment, + type IsolationMode, + type MergeResult, + createIsolation, + mergeDeltaPatches, + readIsolationMode, +} from "./isolation.js"; const MAX_PARALLEL_TASKS = 8; const MAX_CONCURRENCY = 4; @@ -409,6 +418,15 @@ const SubagentParams = Type.Object({ Type.Boolean({ description: "Prompt before running project-local agents. Default: false.", default: false }), ), cwd: Type.Optional(Type.String({ description: "Working directory for the agent process (single mode)" })), + isolated: Type.Optional( + Type.Boolean({ + description: + "Run the subagent in an isolated filesystem (git worktree). " + + "Changes are captured as patches and merged back. " + + "Only available when taskIsolation.mode is configured in settings.", + default: false, + }), + ), }); export default function (pi: ExtensionAPI) { @@ -454,6 +472,10 @@ export default function (pi: ExtensionAPI) { const agents = discovery.agents; const confirmProjectAgents = params.confirmProjectAgents ?? false; + // Resolve isolation mode + const isolationMode = readIsolationMode(); + const useIsolation = Boolean(params.isolated) && isolationMode !== "none"; + const hasChain = (params.chain?.length ?? 0) > 0; const hasTasks = (params.tasks?.length ?? 0) > 0; const hasSingle = Boolean(params.agent && params.task); @@ -668,31 +690,60 @@ export default function (pi: ExtensionAPI) { } if (params.agent && params.task) { - const result = await runSingleAgent( - ctx.cwd, - agents, - params.agent, - params.task, - params.cwd, - undefined, - signal, - onUpdate, - makeDetails("single"), - ); - const isError = result.exitCode !== 0 || result.stopReason === "error" || result.stopReason === "aborted"; - if (isError) { - const errorMsg = - result.errorMessage || result.stderr || getFinalOutput(result.messages) || "(no output)"; + let isolation: IsolationEnvironment | null = null; + let mergeResult: MergeResult | undefined; + try { + const effectiveCwd = params.cwd ?? ctx.cwd; + + if (useIsolation) { + const taskId = crypto.randomUUID(); + isolation = await createIsolation(effectiveCwd, taskId, isolationMode); + } + + const result = await runSingleAgent( + ctx.cwd, + agents, + params.agent, + params.task, + isolation ? isolation.workDir : params.cwd, + undefined, + signal, + onUpdate, + makeDetails("single"), + ); + + // Capture and merge delta if isolated + if (isolation) { + const patches = await isolation.captureDelta(); + if (patches.length > 0) { + mergeResult = await mergeDeltaPatches(effectiveCwd, patches); + } + } + + const isError = result.exitCode !== 0 || result.stopReason === "error" || result.stopReason === "aborted"; + if (isError) { + const errorMsg = + result.errorMessage || result.stderr || getFinalOutput(result.messages) || "(no output)"; + return { + content: [{ type: "text", text: `Agent ${result.stopReason || "failed"}: ${errorMsg}` }], + details: makeDetails("single")([result]), + isError: true, + }; + } + + let outputText = getFinalOutput(result.messages) || "(no output)"; + if (mergeResult && !mergeResult.success) { + outputText += `\n\nāš  Patch merge failed: ${mergeResult.error || "unknown error"}`; + } return { - content: [{ type: "text", text: `Agent ${result.stopReason || "failed"}: ${errorMsg}` }], + content: [{ type: "text", text: outputText }], details: makeDetails("single")([result]), - isError: true, }; + } finally { + if (isolation) { + await isolation.cleanup(); + } } - return { - content: [{ type: "text", text: getFinalOutput(result.messages) || "(no output)" }], - details: makeDetails("single")([result]), - }; } const available = agents.map((a) => `${a.name} (${a.source})`).join(", ") || "none"; diff --git a/src/resources/extensions/subagent/isolation.ts b/src/resources/extensions/subagent/isolation.ts new file mode 100644 index 000000000..9940b8b7b --- /dev/null +++ b/src/resources/extensions/subagent/isolation.ts @@ -0,0 +1,498 @@ +/** + * Task isolation backends for subagent execution. + * + * Provides filesystem isolation via git worktrees or FUSE overlays + * so concurrent subagents don't stomp on each other's files. + * Changes are captured as patches and merged back to the main repo. + */ + +import { execFile as execFileCb } from "node:child_process"; +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; +import { promisify } from "node:util"; + +const execFile = promisify(execFileCb); + +// ============================================================================ +// Types +// ============================================================================ + +export type IsolationMode = "none" | "worktree" | "fuse-overlay"; + +export interface DeltaPatch { + /** Patch file path (for logging/debugging) */ + path: string; + /** Unified diff content */ + content: string; +} + +export interface MergeResult { + success: boolean; + appliedPatches: string[]; + failedPatches: string[]; + error?: string; +} + +export interface IsolationEnvironment { + /** The isolated working directory */ + workDir: string; + /** Teardown the isolation environment */ + cleanup: () => Promise; + /** Capture changes made in the isolated environment */ + captureDelta: () => Promise; +} + +interface Baseline { + stagedDiff: string; + unstagedDiff: string; + untrackedFiles: Array<{ relativePath: string; content: Buffer }>; +} + +// ============================================================================ +// Directory helpers +// ============================================================================ + +function encodeCwd(cwd: string): string { + return cwd.replace(/\//g, "--"); +} + +function getIsolationBaseDir(cwd: string, taskId: string): string { + return path.join(os.homedir(), ".gsd", "wt", encodeCwd(cwd), taskId); +} + +// Track active isolation dirs for cleanup on exit +const activeIsolations = new Set(); +let exitHandlerRegistered = false; + +function registerExitHandler(): void { + if (exitHandlerRegistered) return; + exitHandlerRegistered = true; + + const cleanup = () => { + for (const dir of activeIsolations) { + try { + // Best-effort sync cleanup: remove git worktree + const { execFileSync } = require("node:child_process"); + try { + execFileSync("git", ["worktree", "remove", "--force", dir], { + stdio: "ignore", + timeout: 5000, + }); + } catch { + // Worktree may not exist (FUSE mode), just rm + } + fs.rmSync(dir, { recursive: true, force: true }); + } catch { + // Best effort + } + } + }; + + process.on("exit", cleanup); +} + +// ============================================================================ +// Git helpers +// ============================================================================ + +async function git(args: string[], cwd: string): Promise { + const { stdout } = await execFile("git", args, { + cwd, + maxBuffer: 50 * 1024 * 1024, // 50MB for large diffs + }); + return stdout; +} + +async function gitSilent(args: string[], cwd: string): Promise { + try { + return await git(args, cwd); + } catch { + return ""; + } +} + +// ============================================================================ +// Baseline: capture and apply dirty state +// ============================================================================ + +async function captureBaseline(repoRoot: string): Promise { + // Staged changes + const stagedDiff = await gitSilent(["diff", "--cached", "--binary"], repoRoot); + + // Unstaged changes (tracked files only) + const unstagedDiff = await gitSilent(["diff", "--binary"], repoRoot); + + // Untracked files + const untrackedOutput = await gitSilent( + ["ls-files", "--others", "--exclude-standard", "-z"], + repoRoot, + ); + const untrackedPaths = untrackedOutput + .split("\0") + .filter((p) => p.length > 0); + + const untrackedFiles: Array<{ relativePath: string; content: Buffer }> = []; + for (const relativePath of untrackedPaths) { + const fullPath = path.join(repoRoot, relativePath); + try { + const stat = fs.statSync(fullPath); + if (stat.isFile() && stat.size < 10 * 1024 * 1024) { + // Skip files > 10MB + untrackedFiles.push({ + relativePath, + content: fs.readFileSync(fullPath), + }); + } + } catch { + // Skip unreadable files + } + } + + return { stagedDiff, unstagedDiff, untrackedFiles }; +} + +async function applyBaseline( + worktreeDir: string, + baseline: Baseline, +): Promise { + // Apply staged diff + if (baseline.stagedDiff.trim()) { + const patchPath = path.join(worktreeDir, ".gsd-staged.patch"); + fs.writeFileSync(patchPath, baseline.stagedDiff); + try { + await git(["apply", "--binary", patchPath], worktreeDir); + await git(["add", "-A"], worktreeDir); + } catch { + // Non-fatal: staged diff may not apply cleanly + } finally { + fs.unlinkSync(patchPath); + } + } + + // Apply unstaged diff on top + if (baseline.unstagedDiff.trim()) { + const patchPath = path.join(worktreeDir, ".gsd-unstaged.patch"); + fs.writeFileSync(patchPath, baseline.unstagedDiff); + try { + await git(["apply", "--binary", patchPath], worktreeDir); + } catch { + // Non-fatal: unstaged diff may not apply cleanly + } finally { + fs.unlinkSync(patchPath); + } + } + + // Copy untracked files + for (const file of baseline.untrackedFiles) { + const dest = path.join(worktreeDir, file.relativePath); + const destDir = path.dirname(dest); + fs.mkdirSync(destDir, { recursive: true }); + fs.writeFileSync(dest, file.content); + } + + // Commit the baseline state so captureDeltaPatch can diff against it + // without accidentally including the parent's dirty state in the delta. + await gitSilent(["add", "-A"], worktreeDir); + await gitSilent( + ["commit", "--allow-empty", "-m", "gsd: baseline snapshot"], + worktreeDir, + ); +} + +// ============================================================================ +// Delta capture +// ============================================================================ + +async function captureDeltaPatch( + isolationDir: string, +): Promise { + const patches: DeltaPatch[] = []; + + // Add all changes (tracked + untracked) to index for diffing + await gitSilent(["add", "-A"], isolationDir); + + // Capture the full diff against HEAD + const diff = await gitSilent( + ["diff", "--cached", "--binary", "HEAD"], + isolationDir, + ); + + if (diff.trim()) { + patches.push({ + path: path.join(isolationDir, "delta.patch"), + content: diff, + }); + } + + return patches; +} + +// ============================================================================ +// Worktree backend +// ============================================================================ + +export async function createWorktreeIsolation( + repoRoot: string, + taskId: string, +): Promise { + const worktreeDir = getIsolationBaseDir(repoRoot, taskId); + + registerExitHandler(); + activeIsolations.add(worktreeDir); + + // Create parent directories + fs.mkdirSync(path.dirname(worktreeDir), { recursive: true }); + + // Remove stale worktree if it exists + try { + await git(["worktree", "remove", "--force", worktreeDir], repoRoot); + } catch { + // Doesn't exist, that's fine + } + // Also clean up any leftover directory + fs.rmSync(worktreeDir, { recursive: true, force: true }); + + // Create the worktree + await git( + ["worktree", "add", "--detach", worktreeDir, "HEAD"], + repoRoot, + ); + + // Capture and apply the parent's dirty state + const baseline = await captureBaseline(repoRoot); + await applyBaseline(worktreeDir, baseline); + + return { + workDir: worktreeDir, + + async captureDelta(): Promise { + return captureDeltaPatch(worktreeDir); + }, + + async cleanup(): Promise { + activeIsolations.delete(worktreeDir); + try { + await git( + ["worktree", "remove", "--force", worktreeDir], + repoRoot, + ); + } catch { + // Force remove directory if git worktree remove fails + fs.rmSync(worktreeDir, { recursive: true, force: true }); + } + }, + }; +} + +// ============================================================================ +// FUSE overlay backend (Linux only) +// ============================================================================ + +async function findBinary(name: string): Promise { + try { + const { stdout } = await execFile("which", [name]); + const p = stdout.trim(); + return p || null; + } catch { + return null; + } +} + +export async function createFuseOverlayIsolation( + repoRoot: string, + taskId: string, +): Promise { + const baseDir = getIsolationBaseDir(repoRoot, taskId); + const upperDir = path.join(baseDir, "upper"); + const workDir = path.join(baseDir, "work"); + const mergedDir = path.join(baseDir, "merged"); + + // Check for fuse-overlayfs + const fuseBin = await findBinary("fuse-overlayfs"); + if (!fuseBin) { + // Fall back to worktree + return createWorktreeIsolation(repoRoot, taskId); + } + + registerExitHandler(); + activeIsolations.add(baseDir); + + // Clean up any stale mount/directory + fs.rmSync(baseDir, { recursive: true, force: true }); + + // Create directory structure + fs.mkdirSync(upperDir, { recursive: true }); + fs.mkdirSync(workDir, { recursive: true }); + fs.mkdirSync(mergedDir, { recursive: true }); + + // Mount the overlay + await execFile(fuseBin, [ + "-o", + `lowerdir=${repoRoot},upperdir=${upperDir},workdir=${workDir}`, + mergedDir, + ]); + + // Capture the parent's dirty file set so we can exclude them from the delta. + // Upper dir will contain both parent-dirty files (visible through overlay) and + // subagent-written files ā€” we only want the latter. + const parentDirtyFiles = new Set(); + const parentStatus = await gitSilent(["status", "--porcelain", "-z"], repoRoot); + for (const entry of parentStatus.split("\0").filter(Boolean)) { + // Porcelain format: XY filename (skip 3-char prefix) + const filePath = entry.slice(3); + if (filePath) parentDirtyFiles.add(filePath); + } + + return { + workDir: mergedDir, + + async captureDelta(): Promise { + // Generate patches from upper dir (files actually written by the subagent). + // Exclude files that were already dirty in the parent repo. + const patches: DeltaPatch[] = []; + const diffs: string[] = []; + + const walk = (dir: string, prefix: string) => { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const rel = prefix ? `${prefix}/${entry.name}` : entry.name; + if (entry.isDirectory()) { + walk(path.join(dir, entry.name), rel); + } else if (entry.isFile() && !parentDirtyFiles.has(rel)) { + // This file was written by the subagent, not inherited from parent + diffs.push(rel); + } + } + }; + walk(upperDir, ""); + + if (diffs.length > 0) { + // Use git diff in the merged dir (which has the .git) for only subagent files + const diff = await gitSilent( + ["diff", "--binary", "HEAD", "--", ...diffs], + mergedDir, + ); + if (diff.trim()) { + patches.push({ + path: path.join(mergedDir, "delta.patch"), + content: diff, + }); + } + } + + return patches; + }, + + async cleanup(): Promise { + activeIsolations.delete(baseDir); + try { + // Unmount + const fusermount = (await findBinary("fusermount")) || "fusermount"; + await execFile(fusermount, ["-u", mergedDir]); + } catch { + // Try fusermount3 as fallback + try { + await execFile("fusermount3", ["-u", mergedDir]); + } catch { + // Best effort + } + } + // Remove all dirs + fs.rmSync(baseDir, { recursive: true, force: true }); + }, + }; +} + +// ============================================================================ +// Unified creation +// ============================================================================ + +export async function createIsolation( + repoRoot: string, + taskId: string, + mode: IsolationMode, +): Promise { + switch (mode) { + case "fuse-overlay": + return createFuseOverlayIsolation(repoRoot, taskId); + case "worktree": + return createWorktreeIsolation(repoRoot, taskId); + default: + throw new Error(`Isolation mode "${mode}" requires no isolation environment`); + } +} + +// ============================================================================ +// Patch merge +// ============================================================================ + +export async function mergeDeltaPatches( + repoRoot: string, + patches: DeltaPatch[], +): Promise { + if (patches.length === 0) { + return { success: true, appliedPatches: [], failedPatches: [] }; + } + + // Combine all patches into one + const combined = patches.map((p) => p.content).join("\n"); + const patchFile = path.join( + os.tmpdir(), + `gsd-merge-${Date.now()}.patch`, + ); + + const appliedPatches: string[] = []; + const failedPatches: string[] = []; + + try { + fs.writeFileSync(patchFile, combined); + + // Dry run first + try { + await git( + ["apply", "--check", "--binary", patchFile], + repoRoot, + ); + } catch (err) { + // Dry run failed ā€” patches conflict + for (const p of patches) failedPatches.push(p.path); + return { + success: false, + appliedPatches, + failedPatches, + error: `Patch conflict: ${err instanceof Error ? err.message : String(err)}`, + }; + } + + // Apply for real + await git(["apply", "--binary", patchFile], repoRoot); + for (const p of patches) appliedPatches.push(p.path); + + return { success: true, appliedPatches, failedPatches }; + } finally { + try { + fs.unlinkSync(patchFile); + } catch { + // Best effort + } + } +} + +// ============================================================================ +// Settings reader (reads directly from settings file) +// ============================================================================ + +export function readIsolationMode(): IsolationMode { + try { + const { getAgentDir } = require("@gsd/pi-coding-agent"); + const settingsPath = path.join(getAgentDir(), "settings.json"); + if (!fs.existsSync(settingsPath)) return "none"; + const settings = JSON.parse(fs.readFileSync(settingsPath, "utf-8")); + const mode = settings?.taskIsolation?.mode; + if (mode === "worktree" || mode === "fuse-overlay") return mode; + return "none"; + } catch { + return "none"; + } +} +