From 242bcbef8cb9f3a2b4be76e0ecee4bed0d8d2b91 Mon Sep 17 00:00:00 2001 From: can1357 Date: Wed, 22 Jul 2026 23:54:15 +0200 Subject: [PATCH] revert #6130: doesn't really work well --- packages/coding-agent/CHANGELOG.md | 15 +-- .../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 | 56 ++------- .../src/task/structured-subagent.ts | 25 ---- packages/coding-agent/src/task/types.ts | 115 +++++++++++++----- .../test/task/render-call.test.ts | 21 ++-- .../test/task/structured-subagent.test.ts | 52 -------- .../coding-agent/test/task/task-batch.test.ts | 41 +------ .../test/task/task-progress-render.test.ts | 55 +-------- 11 files changed, 130 insertions(+), 309 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 4e6392672..dac90d031 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -10,6 +10,7 @@ ### Changed - Added `omp-linux-musl-x64` and `omp-linux-musl-arm64` release binaries for Alpine and other musl-based Linux distributions, with automatic musl selection in the binary installer. ([#3367](https://github.com/can1357/oh-my-pi/issues/3367)) +- Edit-tool previews, diff components, and intra-line word highlighting now compute line and word diffs natively, cutting synchronous diff time 2-10x on large inputs (a 50k-line file at 20% edit density drops from ~26s to ~2.4s; see `packages/natives/bench/diff-results.md`) ([#6279](https://github.com/can1357/oh-my-pi/pull/6279) by [@wolfiesch](https://github.com/wolfiesch)). ### Fixed @@ -244,20 +245,6 @@ ## [17.0.3] - 2026-07-17 -### 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). - -### Fixed - -- Fixed `task` rejecting `apply` controls unless `isolated: true`, preventing capture-only intent from silently running in the parent checkout, and kept isolation/capture-only badges visible throughout progress and result rendering. - -### Changed - -- Edit-tool previews, diff components, and intra-line word highlighting now compute line and word diffs natively, cutting synchronous diff time 2-10x on large inputs (a 50k-line file at 20% edit density drops from ~26s to ~2.4s; see `packages/natives/bench/diff-results.md`) ([#6279](https://github.com/can1357/oh-my-pi/pull/6279) by [@wolfiesch](https://github.com/wolfiesch)). - -## [17.0.3] - 2026-07-17 - ### Changed - `omp usage` and the in-session `/usage` view now show the Anthropic organization next to the account for org-scoped credentials (with `--redact` masking applied per part in the CLI, falling back to the org id when no display name is available), attribute "no usage data" rows per organization, and match the "in use by this session" marker by organization so only the active subscription is flagged. The OAuth login success message names the account and organization that was stored — a login landing on an unintended subscription is visible immediately. diff --git a/packages/coding-agent/src/eval/agent-bridge.ts b/packages/coding-agent/src/eval/agent-bridge.ts index 0ebea4e7e..92936b15a 100644 --- a/packages/coding-agent/src/eval/agent-bridge.ts +++ b/packages/coding-agent/src/eval/agent-bridge.ts @@ -7,7 +7,6 @@ import { runStructuredSubagent, StructuredSubagentError, type StructuredSubagentSchemaMode, - toStructuredSubagentIsolationControls, } from "../task/structured-subagent"; import type { AgentProgress, SingleResult } from "../task/types"; import type { NestedRepoPatch } from "../task/worktree"; @@ -15,7 +14,6 @@ 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"; @@ -132,7 +130,14 @@ 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 = toStructuredSubagentIsolationControls(parsed); + 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; 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 9495b7e80..f6adb3bab 100644 --- a/packages/coding-agent/src/prompts/tools/task.md +++ b/packages/coding-agent/src/prompts/tools/task.md @@ -25,8 +25,7 @@ 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 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. + - `isolated`: Run in dedicated worktree, return patches. Destroyed on completion, cannot be addressed afterward. {{/if}} {{else}} - `name`: A stable CamelCase identifier (≤32 chars), used to address the agent (IRC, job ids). Generated automatically if omitted. @@ -35,8 +34,7 @@ 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 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. +- `isolated`: Run in dedicated worktree, return patches. {{/if}} {{/if}} diff --git a/packages/coding-agent/src/task/index.ts b/packages/coding-agent/src/task/index.ts index 3bf5626e1..b1a546310 100644 --- a/packages/coding-agent/src/task/index.ts +++ b/packages/coding-agent/src/task/index.ts @@ -33,7 +33,6 @@ import { canSpawnAtDepth, getTaskSchema, type SingleResult, - type TaskIsolationControls, type TaskItem, type TaskParams, type TaskToolDetails, @@ -49,12 +48,7 @@ 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, - toStructuredSubagentIsolationControls, -} from "./structured-subagent"; +import { resolveEffectiveSubagentPolicy, runStructuredSubagent, StructuredSubagentError } from "./structured-subagent"; function renderSubagentUserPrompt(assignment: string): string { return prompt.render(subagentUserPromptTemplate, { @@ -275,22 +269,10 @@ 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 isolation controls - * are only materialized when the caller sent them — `#runSpawn` + * provided, otherwise the single top-level spawn. The flat form's `isolated` + * flag is only materialized when the caller sent one — `#runSpawn` * distinguishes an absent key from an explicit value. */ function resolveSpawnItems(params: TaskParams): TaskItem[] { @@ -300,7 +282,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; - copyTaskIsolationControls(item, params); + if ("isolated" in params) item.isolated = params.isolated; return [item]; } @@ -310,8 +292,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 absent isolation controls from explicit values. Per-item - * controls (batch form) win over top-level controls (flat compatibility form). + * distinguishes an absent `isolated` from an explicit one. The item's + * `isolated` (batch form) wins over the top-level flag (flat form). */ function spawnParamsFor(params: TaskParams, item: TaskItem, defaultAgent: string): TaskParams { const spawn: TaskParams = { agent: item.agent?.trim() || defaultAgent }; @@ -320,7 +302,11 @@ 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; - copyTaskIsolationControls(spawn, item, params); + if (item.isolated !== undefined) { + spawn.isolated = item.isolated; + } else if ("isolated" in params) { + spawn.isolated = params.isolated; + } return spawn; } @@ -620,7 +606,6 @@ export class TaskTool implements AgentTool | undefined, - index: number, - theme: Theme, -): void { - if (lines.length === 0) return; - const input = Array.isArray(args?.tasks) ? args.tasks[index] : args; - lines[0] += isolationBadge(input, theme); -} - /** * Render the call preview lines for the single spawned agent. The * args stream in token by token, so every field access is defensive. @@ -784,7 +761,9 @@ function renderTaskItemLines(tasks: TaskItem[] | undefined, theme: Theme): strin line += `: ${theme.fg("muted", previewLine(brief, 64))}`; } line += agentTypeBadge(item?.agent, theme); - line += isolationBadge(item, theme); + if (item?.isolated === true) { + line += theme.fg("dim", " [isolated]"); + } lines.push(line); } if (cap < tasks.length) { @@ -850,7 +829,6 @@ function createMarkdownSectionRenderer(text: string, theme: Theme): AssignmentSe */ export function renderCall(args: TaskParams, options: TaskRenderOptions, theme: Theme): Component { const showIsolated = "isolated" in args && args.isolated === true; - const captureOnly = showIsolated && args.apply === false; // Dispatch glyph from the first frame: spawning is non-blocking, so a // pending/hourglass icon would misread the call as something the turn // waits on. @@ -888,7 +866,7 @@ export function renderCall(args: TaskParams, options: TaskRenderOptions, theme: return { header, - headerMeta: captureOnly ? "isolated · capture-only" : showIsolated ? "isolated" : undefined, + headerMeta: showIsolated ? "isolated" : undefined, sections, state: "pending", borderColor: "borderMuted", @@ -1504,27 +1482,17 @@ export function renderResult( const agentLabel = formatAgentHeaderLabel(args); const assignmentSection = createAssignmentSectionRenderer(args, theme); const contextSection = createContextSectionRenderer(args, theme); - const flatIsolationMeta = Array.isArray(args?.tasks) ? undefined : isolationMeta(args); if (!details) { const text = result.content.find(c => c.type === "text")?.text || ""; const errored = result.isError === true; const header = errored - ? renderStatusLine( - { - icon: "error", - title: "Task", - description: agentLabel, - meta: flatIsolationMeta ? [flatIsolationMeta] : undefined, - }, - theme, - ) + ? renderStatusLine({ icon: "error", title: "Task", description: agentLabel }, theme) : renderStatusLine( { iconOverride: theme.styledSymbol("status.done", "accent"), title: "Task", description: agentLabel, - meta: flatIsolationMeta ? [flatIsolationMeta] : undefined, }, theme, ); @@ -1584,7 +1552,7 @@ export function renderResult( ? theme.styledSymbol("status.done", "accent") : undefined, title: "Task", - meta: [metaLabel, flatIsolationMeta].filter((label): label is string => label !== undefined), + meta: metaLabel ? [metaLabel] : undefined, }, theme, ); @@ -1609,17 +1577,13 @@ export function renderResult( lines.push(formatHiddenProgressLine(ordered.slice(0, ordered.length - visible.length), theme)); } for (const progress of visible) { - const rendered = renderAgentProgress(progress, "", " ", expanded, theme, spinnerFrame, frozen); - appendIsolationBadge(rendered, args, progress.index, theme); - lines.push(...rendered); + lines.push(...renderAgentProgress(progress, "", " ", expanded, theme, spinnerFrame, frozen)); } } else if (details.results && details.results.length > 0) { const ordered = orderResultsForDisplay(details.results); const visible = expanded ? ordered : selectCollapsedResults(ordered); for (const res of visible) { - const rendered = renderAgentResult(res, "", " ", expanded, theme); - appendIsolationBadge(rendered, args, res.index, theme); - lines.push(...rendered); + lines.push(...renderAgentResult(res, "", " ", expanded, theme)); } if (visible.length < ordered.length) { const hint = formatExpandHint(theme, false, true); @@ -1638,9 +1602,7 @@ export function renderResult( ) : []; for (const progress of supplementalProgress) { - const rendered = renderAgentProgress(progress, "", " ", expanded, theme, spinnerFrame, frozen); - appendIsolationBadge(rendered, args, progress.index, theme); - lines.push(...rendered); + lines.push(...renderAgentProgress(progress, "", " ", expanded, theme, spinnerFrame, frozen)); } const summaryParts: string[] = []; diff --git a/packages/coding-agent/src/task/structured-subagent.ts b/packages/coding-agent/src/task/structured-subagent.ts index b89900db7..e8a18ea05 100644 --- a/packages/coding-agent/src/task/structured-subagent.ts +++ b/packages/coding-agent/src/task/structured-subagent.ts @@ -68,28 +68,6 @@ export interface StructuredSubagentIsolationControls { apply?: boolean; } -/** Model-facing isolation controls accepted by task and eval adapters. */ -export interface StructuredSubagentIsolationInput { - isolated?: boolean; - apply?: boolean; - merge?: boolean; -} - -/** Translate model-facing isolation flags into the shared executor policy. */ -export function toStructuredSubagentIsolationControls( - input: StructuredSubagentIsolationInput, -): StructuredSubagentIsolationControls | undefined { - const requested = input.isolated !== undefined ? input.isolated : undefined; - const apply = input.apply !== undefined ? input.apply : undefined; - const merge = input.merge === false ? "patch" : undefined; - if (requested === undefined && apply === undefined && merge === undefined) return undefined; - return { - ...(requested !== undefined ? { requested } : {}), - ...(merge !== undefined ? { merge } : {}), - ...(apply !== undefined ? { apply } : {}), - }; -} - /** Identity and presentation metadata supplied by the calling surface. */ export interface StructuredSubagentIdentity { /** A previously reserved output/registry id. */ @@ -265,9 +243,6 @@ export async function resolveEffectiveSubagentPolicy( const agentName = request.agent?.trim() || spawnPolicy.defaultAgent; const planMode = request.session.getPlanModeState?.()?.enabled === true; assertPlanControlsAllowed(request, planMode); - if (request.isolation?.apply !== undefined && request.isolation.requested !== true) { - throw new StructuredSubagentError("preflight", "Subagent `apply` control requires `isolated: true`."); - } assertDepthAndSpawnAllowed(request, agentName); const discovery = await discoverAgents(request.session.cwd); diff --git a/packages/coding-agent/src/task/types.ts b/packages/coding-agent/src/task/types.ts index fa153208c..62007fd3d 100644 --- a/packages/coding-agent/src/task/types.ts +++ b/packages/coding-agent/src/task/types.ts @@ -109,31 +109,26 @@ export const LABEL_MAX = 80; // Keep this explicit: ArkType serializes `unknown` as a boolean subschema, which llama.cpp grammars reject. const outputSchemaInputSchema = type("object | boolean | string | null"); -const TASK_ITEM_FIELDS = { +export const taskItemSchema = type({ + "name?": "string", + agent: "string = 'task'", + task: "string", + "outputSchema?": outputSchemaInputSchema, + "schemaMode?": '"permissive" | "strict"', + "+": "delete", +}); +const taskItemSchemaIsolated = type({ "name?": "string", agent: "string = 'task'", task: "string", "outputSchema?": outputSchemaInputSchema, "schemaMode?": '"permissive" | "strict"', -} as const; -const TASK_ISOLATION_FIELDS = { "isolated?": "boolean", - "apply?": "boolean", -} as const; - -export const taskItemSchema = type({ ...TASK_ITEM_FIELDS, "+": "delete" }); -const taskItemSchemaIsolated = type({ ...TASK_ITEM_FIELDS, ...TASK_ISOLATION_FIELDS, "+": "delete" }); - -/** Model-facing controls for one task spawn's isolated worktree. */ -export interface TaskIsolationControls { - /** Run this spawn in an isolated worktree. */ - isolated?: boolean; - /** Apply isolated changes to the parent checkout; defaults to true. */ - apply?: boolean; -} + "+": "delete", +}); /** Single task item. Fields are optional defensively: args stream in token by token. */ -export interface TaskItem extends TaskIsolationControls { +export interface TaskItem { /** Stable agent name; becomes the registry/IRC id. Default = generated AdjectiveNoun. */ name?: string; /** Agent type to run this item (e.g. "scout"). Defaults to the spawn policy's default agent. */ @@ -144,10 +139,27 @@ export interface TaskItem extends TaskIsolationControls { outputSchema?: unknown; /** Validation behavior for a caller-provided or inherited output schema. */ schemaMode?: "permissive" | "strict"; + /** Run this spawn in an isolated worktree (batch form; flat form carries it top-level). */ + isolated?: boolean; } -export const taskSchema = taskItemSchemaIsolated; -const taskSchemaNoIsolation = taskItemSchema; +export const taskSchema = type({ + "name?": "string", + agent: "string = 'task'", + task: "string", + "outputSchema?": outputSchemaInputSchema, + "schemaMode?": '"permissive" | "strict"', + "isolated?": "boolean", + "+": "delete", +}); +const taskSchemaNoIsolation = type({ + "name?": "string", + agent: "string = 'task'", + task: "string", + "outputSchema?": outputSchemaInputSchema, + "schemaMode?": '"permissive" | "strict"', + "+": "delete", +}); const taskSchemaBatch = type({ context: "string", tasks: taskItemSchemaIsolated.array(), @@ -176,25 +188,60 @@ function taskAgentSchemaRule(defaultAgent: string): string { return "string"; } -function createTaskItemSchema(agent: string, isolationEnabled: boolean): BaseType { - return type.raw({ - ...TASK_ITEM_FIELDS, - agent, - ...(isolationEnabled ? TASK_ISOLATION_FIELDS : {}), - "+": "delete", - }); -} - function createTaskSchema(options: { isolationEnabled: boolean; batchEnabled: boolean; defaultAgent: string; }): BaseType { - const item = createTaskItemSchema(taskAgentSchemaRule(options.defaultAgent), options.isolationEnabled); - if (!options.batchEnabled) return item; + const agent = taskAgentSchemaRule(options.defaultAgent); + if (options.batchEnabled) { + if (options.isolationEnabled) { + const item = type.raw({ + "name?": "string", + agent, + task: "string", + "outputSchema?": outputSchemaInputSchema, + "schemaMode?": '"permissive" | "strict"', + "isolated?": "boolean", + "+": "delete", + }); + return type.raw({ + context: "string", + tasks: item.array(), + "+": "delete", + }); + } + const item = type.raw({ + "name?": "string", + agent, + task: "string", + "outputSchema?": outputSchemaInputSchema, + "schemaMode?": '"permissive" | "strict"', + "+": "delete", + }); + return type.raw({ + context: "string", + tasks: item.array(), + "+": "delete", + }); + } + if (options.isolationEnabled) { + return type.raw({ + "name?": "string", + agent, + task: "string", + "outputSchema?": outputSchemaInputSchema, + "schemaMode?": '"permissive" | "strict"', + "isolated?": "boolean", + "+": "delete", + }); + } return type.raw({ - context: "string", - tasks: item.array(), + "name?": "string", + agent, + task: "string", + "outputSchema?": outputSchemaInputSchema, + "schemaMode?": '"permissive" | "strict"', "+": "delete", }); } @@ -229,7 +276,7 @@ export function getTaskSchema(options: { * otherwise); runtime stays permissive so internal callers and stale * transcripts using the flat form keep working under either setting. */ -export interface TaskParams extends TaskIsolationControls { +export interface TaskParams { /** Stable agent name (flat form). */ name?: string; /** Agent type to spawn (flat form); omitted values resolve from the session spawn policy. */ @@ -244,6 +291,8 @@ export interface TaskParams extends TaskIsolationControls { tasks?: TaskItem[]; /** Batch form: shared background prepended to every assignment; required by the batch schema. */ context?: string; + /** Run in an isolated worktree (flat form; per-item in batch form). */ + isolated?: boolean; } /** diff --git a/packages/coding-agent/test/task/render-call.test.ts b/packages/coding-agent/test/task/render-call.test.ts index 181e0b748..b3c8ded82 100644 --- a/packages/coding-agent/test/task/render-call.test.ts +++ b/packages/coding-agent/test/task/render-call.test.ts @@ -88,25 +88,20 @@ describe("task renderer: streaming call preview", () => { expect(expanded).toContain("Step 6"); }); - it("surfaces isolated and capture-only intent in call previews", () => { - const isolated: TaskParams = { + it("surfaces the isolation flag in the header bar", () => { + const args: TaskParams = { agent: "task", isolated: true, name: "Only", task: "...", }; - const isolatedLines = render(isolated).split("\n"); - expect(isolatedLines[0]).toContain("isolated"); - expect(isolatedLines[0]).not.toContain("capture-only"); + const out = render(args); + const lines = out.split("\n"); - 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]"); + 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"); }); // 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 148c196fa..fc28abf50 100644 --- a/packages/coding-agent/test/task/structured-subagent.test.ts +++ b/packages/coding-agent/test/task/structured-subagent.test.ts @@ -17,7 +17,6 @@ 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"; @@ -398,55 +397,4 @@ 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("rejects apply controls unless isolation is explicitly requested", async () => { - const discover = vi.spyOn(discoveryModule, "discoverAgents"); - for (const isolation of [{ apply: false }, { requested: false, apply: false }, { apply: true }]) { - await expect(resolveEffectiveSubagentPolicy(request({ isolation }))).rejects.toThrow( - "Subagent `apply` control requires `isolated: true`.", - ); - } - expect(discover).not.toHaveBeenCalled(); - }); - - 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 6ac24e414..ed8db870a 100644 --- a/packages/coding-agent/test/task/task-batch.test.ts +++ b/packages/coding-agent/test/task/task-batch.test.ts @@ -125,30 +125,19 @@ describe("task.batch schema gating", () => { expect(items?.properties?.schemaMode).toBeDefined(); }); - it("places isolation controls per item in the batch shape and top-level in the flat shape", async () => { + it("places isolated per item in the batch shape when isolation is enabled", async () => { mockDiscovery(); - const batch = await TaskTool.create( + const tool = await TaskTool.create( createSession({ settings: { "task.batch": true, "task.isolation.mode": "auto" } }), ); - const batchProperties = getSchemaProperties(batch); - expect(batchProperties.isolated).toBeUndefined(); - expect(batchProperties.apply).toBeUndefined(); - const items = (batchProperties.tasks as { items?: { properties?: Record } }).items; + const properties = getSchemaProperties(tool); + expect(properties.isolated).toBeUndefined(); + const items = (properties.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 controls from the dynamic batch schema in plan mode", async () => { + it("hides isolation from the dynamic batch schema in plan mode", async () => { mockDiscovery(); const tool = await TaskTool.create( createSession({ @@ -159,9 +148,7 @@ 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 () => { @@ -225,22 +212,6 @@ describe("task.batch validation", () => { expect(text).toContain("Missing `context`"); }); - it("rejects apply without effective isolation before spawning", async () => { - const runSubprocess = vi.spyOn(executorModule, "runSubprocess"); - const flat = await executeText( - { agent: "task", task: "Work.", apply: false }, - { "task.batch": false, "task.isolation.mode": "worktree" }, - ); - expect(flat).toContain("`apply` control requires `isolated: true`"); - - const batch = await executeText( - { context: "Shared.", tasks: [{ task: "Work.", isolated: false, apply: false }] }, - { "task.batch": true, "task.isolation.mode": "worktree" }, - ); - expect(batch).toContain("`apply` control requires `isolated: true`"); - expect(runSubprocess).not.toHaveBeenCalled(); - }); - it("rejects duplicate provided names case-insensitively", async () => { const text = await executeText( { diff --git a/packages/coding-agent/test/task/task-progress-render.test.ts b/packages/coding-agent/test/task/task-progress-render.test.ts index d29d38495..e9fc8ef42 100644 --- a/packages/coding-agent/test/task/task-progress-render.test.ts +++ b/packages/coding-agent/test/task/task-progress-render.test.ts @@ -4,7 +4,7 @@ import type { SettingPath, SettingValue } from "@oh-my-pi/pi-coding-agent/config import { resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { getThemeByName, setThemeInstance } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; import { taskToolRenderer } from "@oh-my-pi/pi-coding-agent/task/renderer"; -import type { AgentProgress, SingleResult, TaskParams, TaskToolDetails } from "@oh-my-pi/pi-coding-agent/task/types"; +import type { AgentProgress, SingleResult, TaskToolDetails } from "@oh-my-pi/pi-coding-agent/task/types"; function runningProgress(overrides: Partial = {}): AgentProgress { return { @@ -152,59 +152,6 @@ describe("task progress rendering", () => { expect(genericRow).not.toContain(`${theme.format.bracketLeft}task${theme.format.bracketRight}`); }); - it("keeps capture-only metadata through progress and result rendering", async () => { - const theme = (await getThemeByName("dark"))!; - setThemeInstance(theme); - const flatArgs: TaskParams = { - agent: "task", - task: "Capture the patch.", - isolated: true, - apply: false, - }; - const flatProgress = taskToolRenderer.renderResult( - { content: [{ type: "text", text: "" }], details: detailsFor(runningProgress({ id: "FlatCapture" })) }, - { expanded: false, isPartial: true, spinnerFrame: 0 }, - theme, - flatArgs, - ); - expect(Bun.stripANSI(flatProgress.render(120).join("\n")).split("\n")[0]).toContain("capture-only"); - - const batchArgs: TaskParams = { - context: "Shared.", - tasks: [ - { name: "Captured", task: "Capture.", isolated: true, apply: false }, - { name: "Applied", task: "Apply.", isolated: true }, - ], - }; - const progressDetails: TaskToolDetails = { - projectAgentsDir: null, - results: [], - totalDurationMs: 0, - progress: [runningProgress({ index: 0, id: "Captured" }), runningProgress({ index: 1, id: "Applied" })], - }; - const progressComponent = taskToolRenderer.renderResult( - { content: [{ type: "text", text: "" }], details: progressDetails }, - { expanded: false, isPartial: true, spinnerFrame: 0 }, - theme, - batchArgs, - ); - expect(Bun.stripANSI(findRow(progressComponent, "Captured"))).toContain("[isolated, capture-only]"); - expect(Bun.stripANSI(findRow(progressComponent, "Applied"))).toContain("[isolated]"); - - const resultDetails: TaskToolDetails = { - projectAgentsDir: null, - results: [finishedResult({ index: 0, id: "Captured" })], - totalDurationMs: 1, - }; - const resultComponent = taskToolRenderer.renderResult( - { content: [{ type: "text", text: "" }], details: resultDetails }, - { expanded: false, isPartial: false }, - theme, - batchArgs, - ); - expect(Bun.stripANSI(findRow(resultComponent, "Captured"))).toContain("[isolated, capture-only]"); - }); - it("shows the spawn count without a joined agent-type list in the header", async () => { const theme = (await getThemeByName("dark"))!; const details: TaskToolDetails = {