From 6f75cf0d5dbdc25fae7b5b5729021c453070b678 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Korm=C3=A1kur?= Date: Mon, 20 Jul 2026 18:17:35 +0000 Subject: [PATCH] feat(coding-agent): add task tool capture-only apply control Add an optional `apply` parameter to the `task` tool so `isolated: true, apply: false` captures patch/branch artifacts without applying changes to the parent checkout. Available as a flat top-level control and per `tasks[]` item. Shares the task/eval isolation-to-executor translation via a single `toStructuredSubagentIsolationControls` adapter. --- packages/coding-agent/CHANGELOG.md | 4 + .../coding-agent/src/eval/agent-bridge.ts | 11 +- .../coding-agent/src/prompts/tools/task.md | 6 +- packages/coding-agent/src/task/index.ts | 42 +++++-- packages/coding-agent/src/task/render.ts | 5 +- .../src/task/structured-subagent.ts | 22 ++++ packages/coding-agent/src/task/types.ts | 115 +++++------------- .../test/task/render-call.test.ts | 21 ++-- .../test/task/structured-subagent.test.ts | 42 +++++++ .../coding-agent/test/task/task-batch.test.ts | 25 +++- 10 files changed, 172 insertions(+), 121 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 6f32dc885..793f00aa1 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Added + +- Added an optional `apply` control to the `task` tool: with `isolated: true, apply: false`, a subagent runs in its dedicated worktree and its patch/branch artifacts are captured without applying changes to the parent checkout. `apply` defaults to `true`. Available both as a flat top-level control and per `tasks[]` item (per-item value wins). + ## [17.0.5] - 2026-07-18 ### Added diff --git a/packages/coding-agent/src/eval/agent-bridge.ts b/packages/coding-agent/src/eval/agent-bridge.ts index 92936b15a..0ebea4e7e 100644 --- a/packages/coding-agent/src/eval/agent-bridge.ts +++ b/packages/coding-agent/src/eval/agent-bridge.ts @@ -7,6 +7,7 @@ import { runStructuredSubagent, StructuredSubagentError, type StructuredSubagentSchemaMode, + toStructuredSubagentIsolationControls, } from "../task/structured-subagent"; import type { AgentProgress, SingleResult } from "../task/types"; import type { NestedRepoPatch } from "../task/worktree"; @@ -14,6 +15,7 @@ import type { ToolSession } from "../tools"; import { ToolError } from "../tools/tool-errors"; import { withBridgeTimeoutPause } from "./bridge-timeout"; import type { JsStatusEvent } from "./js/shared/types"; + // Import review tools for side effects (registers subagent tool handlers). import "../tools/review"; @@ -130,14 +132,7 @@ export async function runEvalAgent(args: unknown, options: EvalAgentBridgeOption `agent() blocked: turn token budget exhausted (${turnBudget.spent}/${turnBudget.total} output tokens). Raise or drop the +Nk! ceiling to continue.`, ); } - const isolation = - Object.hasOwn(parsed, "isolated") || Object.hasOwn(parsed, "apply") || Object.hasOwn(parsed, "merge") - ? { - ...(parsed.isolated !== undefined ? { requested: parsed.isolated } : {}), - ...(parsed.merge === false ? { merge: "patch" as const } : {}), - ...(parsed.apply !== undefined ? { apply: parsed.apply } : {}), - } - : undefined; + const isolation = toStructuredSubagentIsolationControls(parsed); try { const execution = await withBridgeTimeoutPause( diff --git a/packages/coding-agent/src/prompts/tools/task.md b/packages/coding-agent/src/prompts/tools/task.md index f6adb3bab..9495b7e80 100644 --- a/packages/coding-agent/src/prompts/tools/task.md +++ b/packages/coding-agent/src/prompts/tools/task.md @@ -25,7 +25,8 @@ Agents marked BLOCKING run inline — results return in this call; non-blocking - `outputSchema`: Invocation-specific JSON Schema. Overrides the selected agent and parent-session schemas. - `schemaMode`: `"permissive"` (default) accepts a retry-exhausted invalid result with a warning; `"strict"` fails it. {{#if isolationEnabled}} - - `isolated`: Run in dedicated worktree, return patches. Destroyed on completion, cannot be addressed afterward. + - `isolated`: Run in a dedicated worktree. The worktree is destroyed on completion and cannot be addressed afterward. + - `apply`: With `isolated: true`, apply captured changes to the parent checkout. Defaults to `true`; `false` preserves patch or branch artifacts without modifying the parent. {{/if}} {{else}} - `name`: A stable CamelCase identifier (≤32 chars), used to address the agent (IRC, job ids). Generated automatically if omitted. @@ -34,7 +35,8 @@ Agents marked BLOCKING run inline — results return in this call; non-blocking - `outputSchema`: Invocation-specific JSON Schema. Overrides the selected agent and parent-session schemas. - `schemaMode`: `"permissive"` (default) accepts a retry-exhausted invalid result with a warning; `"strict"` fails it. {{#if isolationEnabled}} -- `isolated`: Run in dedicated worktree, return patches. +- `isolated`: Run in a dedicated worktree. +- `apply`: With `isolated: true`, apply captured changes to the parent checkout. Defaults to `true`; `false` preserves patch or branch artifacts without modifying the parent. {{/if}} {{/if}} diff --git a/packages/coding-agent/src/task/index.ts b/packages/coding-agent/src/task/index.ts index 3ffbe3c57..c5df475f2 100644 --- a/packages/coding-agent/src/task/index.ts +++ b/packages/coding-agent/src/task/index.ts @@ -33,6 +33,7 @@ import { canSpawnAtDepth, getTaskSchema, type SingleResult, + type TaskIsolationControls, type TaskItem, type TaskParams, type TaskToolDetails, @@ -48,7 +49,12 @@ import { AgentOutputManager } from "./output-manager"; import { mapWithConcurrencyLimitAllSettled, Semaphore } from "./parallel"; import { renderResult, renderCall as renderTaskCall } from "./render"; import { repairTaskParams } from "./repair-args"; -import { resolveEffectiveSubagentPolicy, runStructuredSubagent, StructuredSubagentError } from "./structured-subagent"; +import { + resolveEffectiveSubagentPolicy, + runStructuredSubagent, + StructuredSubagentError, + toStructuredSubagentIsolationControls, +} from "./structured-subagent"; function renderSubagentUserPrompt(assignment: string): string { return prompt.render(subagentUserPromptTemplate, { @@ -269,10 +275,22 @@ function validateSpawnParams(params: TaskParams, batchEnabled: boolean): string return undefined; } +/** Copy the first explicitly supplied value for each task isolation control. */ +function copyTaskIsolationControls( + target: TaskIsolationControls, + primary: TaskIsolationControls, + fallback?: TaskIsolationControls, +): void { + if (Object.hasOwn(primary, "isolated")) target.isolated = primary.isolated; + else if (fallback && Object.hasOwn(fallback, "isolated")) target.isolated = fallback.isolated; + if (Object.hasOwn(primary, "apply")) target.apply = primary.apply; + else if (fallback && Object.hasOwn(fallback, "apply")) target.apply = fallback.apply; +} + /** * Normalize a validated call into its spawn list: the `tasks[]` batch when - * provided, otherwise the single top-level spawn. The flat form's `isolated` - * flag is only materialized when the caller sent one — `#runSpawn` + * provided, otherwise the single top-level spawn. The flat form's isolation controls + * are is only materialized when the caller sent one — `#runSpawn` * distinguishes an absent key from an explicit value. */ function resolveSpawnItems(params: TaskParams): TaskItem[] { @@ -282,7 +300,7 @@ function resolveSpawnItems(params: TaskParams): TaskItem[] { const item: TaskItem = { name: params.name, agent: params.agent, task: params.task }; if ("outputSchema" in params) item.outputSchema = params.outputSchema; if ("schemaMode" in params) item.schemaMode = params.schemaMode; - if ("isolated" in params) item.isolated = params.isolated; + copyTaskIsolationControls(item, params); return [item]; } @@ -292,8 +310,8 @@ function resolveSpawnItems(params: TaskParams): TaskItem[] { * the item's own value, else `defaultAgent` from the session spawn policy. * `tasks` never leaks into a spawn; the shared `context` rides along * unchanged. Keys are only materialized when present — `#runSpawn` - * distinguishes an absent `isolated` from an explicit one. The item's - * `isolated` (batch form) wins over the top-level flag (flat form). + * distinguishes absent isolation controls from explicit values. Per-item + * controls (batch form) win over top-level controls (flat compatibility form). */ function spawnParamsFor(params: TaskParams, item: TaskItem, defaultAgent: string): TaskParams { const spawn: TaskParams = { agent: item.agent?.trim() || defaultAgent }; @@ -302,11 +320,7 @@ function spawnParamsFor(params: TaskParams, item: TaskItem, defaultAgent: string if (params.context !== undefined) spawn.context = params.context; if ("outputSchema" in item) spawn.outputSchema = item.outputSchema; if ("schemaMode" in item) spawn.schemaMode = item.schemaMode; - if (item.isolated !== undefined) { - spawn.isolated = item.isolated; - } else if ("isolated" in params) { - spawn.isolated = params.isolated; - } + copyTaskIsolationControls(spawn, item, params); return spawn; } @@ -595,6 +609,7 @@ export class TaskTool implements AgentTool { expect(expanded).toContain("Step 6"); }); - it("surfaces the isolation flag in the header bar", () => { - const args: TaskParams = { + it("surfaces isolated and capture-only intent in call previews", () => { + const isolated: TaskParams = { agent: "task", isolated: true, name: "Only", task: "...", }; - const out = render(args); - const lines = out.split("\n"); + const isolatedLines = render(isolated).split("\n"); + expect(isolatedLines[0]).toContain("isolated"); + expect(isolatedLines[0]).not.toContain("capture-only"); - expect(out).toContain("Only"); - // Isolation is surfaced as header meta in the frame's top bar (first line), - // not as a trailing child row under the task list. - expect(lines[0]).toContain("isolated"); + const capturedLines = render({ ...isolated, apply: false }).split("\n"); + expect(capturedLines[0]).toContain("isolated · capture-only"); + + const batch = render({ + context: "ctx", + tasks: [{ name: "Captured", task: "inspect", isolated: true, apply: false }], + }); + expect(batch).toContain("[isolated, capture-only]"); }); // The batch schema streams `context` before `tasks`, and `renderResult` diff --git a/packages/coding-agent/test/task/structured-subagent.test.ts b/packages/coding-agent/test/task/structured-subagent.test.ts index fc28abf50..ccb226ce2 100644 --- a/packages/coding-agent/test/task/structured-subagent.test.ts +++ b/packages/coding-agent/test/task/structured-subagent.test.ts @@ -17,6 +17,7 @@ import { runStructuredSubagent, StructuredSubagentError, type StructuredSubagentRequest, + toStructuredSubagentIsolationControls, } from "@oh-my-pi/pi-coding-agent/task/structured-subagent"; import type { AgentDefinition, SingleResult } from "@oh-my-pi/pi-coding-agent/task/types"; import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools"; @@ -397,4 +398,45 @@ describe("structured subagent primitive", () => { expect(await fs.stat(artifactsDir ?? "")).toBeDefined(); await fs.rm(settled.artifactsDir, { recursive: true, force: true }); }); + + it("maps shared isolation controls and keeps apply enabled by default", async () => { + mockDiscovery(); + expect(toStructuredSubagentIsolationControls({ isolated: true, apply: false, merge: false })).toEqual({ + requested: true, + merge: "patch", + apply: false, + }); + expect(toStructuredSubagentIsolationControls({})).toBeUndefined(); + + const policy = await resolveEffectiveSubagentPolicy( + request({ session: session({ isolationMode: "worktree" }), isolation: { requested: true } }), + ); + expect(policy.applyChanges).toBe(true); + }); + + it("retains successful isolated captures without merging when apply is false", async () => { + mockDiscovery(); + let artifactsDir: string | undefined; + vi.spyOn(isolationRunner, "prepareIsolationContext").mockResolvedValue({ repoRoot: "/tmp" } as never); + vi.spyOn(isolationRunner, "runIsolatedSubprocess").mockImplementation(async ({ baseOptions }) => { + artifactsDir = baseOptions.artifactsDir; + return { ...result(), patchPath: "/recovery/Worker.patch" }; + }); + const merge = vi.spyOn(isolationRunner, "mergeIsolatedChanges"); + + const settled = await runStructuredSubagent( + request({ + session: session({ isolationMode: "worktree" }), + isolation: { requested: true, apply: false }, + }), + ); + + expect(merge).not.toHaveBeenCalled(); + expect(settled.changesApplied).toBeNull(); + expect(settled.mergeSummary).toContain("apply=false"); + expect(settled.mergeSummary).toContain("Not applied"); + expect(artifactsDirsFromRegistry()).toContain(settled.artifactsDir); + expect(await fs.stat(artifactsDir ?? "")).toBeDefined(); + await fs.rm(settled.artifactsDir, { recursive: true, force: true }); + }); }); diff --git a/packages/coding-agent/test/task/task-batch.test.ts b/packages/coding-agent/test/task/task-batch.test.ts index 9af6cb447..8a26e4675 100644 --- a/packages/coding-agent/test/task/task-batch.test.ts +++ b/packages/coding-agent/test/task/task-batch.test.ts @@ -125,19 +125,30 @@ describe("task.batch schema gating", () => { expect(items?.properties?.schemaMode).toBeDefined(); }); - it("places isolated per item in the batch shape when isolation is enabled", async () => { + it("places isolation controls per item in the batch shape and top-level in the flat shape", async () => { mockDiscovery(); - const tool = await TaskTool.create( + const batch = await TaskTool.create( createSession({ settings: { "task.batch": true, "task.isolation.mode": "auto" } }), ); - const properties = getSchemaProperties(tool); - expect(properties.isolated).toBeUndefined(); - const items = (properties.tasks as { items?: { properties?: Record } }).items; + const batchProperties = getSchemaProperties(batch); + expect(batchProperties.isolated).toBeUndefined(); + expect(batchProperties.apply).toBeUndefined(); + const items = (batchProperties.tasks as { items?: { properties?: Record } }).items; expect(items?.properties?.isolated).toBeDefined(); + expect(items?.properties?.apply).toBeDefined(); + expect(batch.description).toContain("`apply`"); + expect(batch.description).toContain("without modifying the parent"); + + const flat = await TaskTool.create( + createSession({ settings: { "task.batch": false, "task.isolation.mode": "auto" } }), + ); + const flatProperties = getSchemaProperties(flat); + expect(flatProperties.isolated).toBeDefined(); + expect(flatProperties.apply).toBeDefined(); }); - it("hides isolation from the dynamic batch schema in plan mode", async () => { + it("hides isolation controls from the dynamic batch schema in plan mode", async () => { mockDiscovery(); const tool = await TaskTool.create( createSession({ @@ -148,7 +159,9 @@ describe("task.batch schema gating", () => { const properties = getSchemaProperties(tool); const items = (properties.tasks as { items?: { properties?: Record } }).items; expect(items?.properties?.isolated).toBeUndefined(); + expect(items?.properties?.apply).toBeUndefined(); expect(tool.description).not.toContain("`isolated`"); + expect(tool.description).not.toContain("`apply`"); }); it("exposes outputSchema but never the stale schema field", async () => {