smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

revert #6130: doesn't really work well

Browse Source
This commit is contained in:
can1357
2026-07-22 23:54:15 +02:00
parent 59877a01bd
commit 242bcbef8c
11 changed files with 130 additions and 309 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/test/task/render-call.test.ts
+8 -13
View File
@@ -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`
packages/coding-agent/test/task/structured-subagent.test.ts
-52
View File
@@ -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 });
});
});
packages/coding-agent/test/task/task-batch.test.ts
+6 -35
View File
@@ -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<string, unknown> } }).items;
const properties = getSchemaProperties(tool);
expect(properties.isolated).toBeUndefined();
const items = (properties.tasks as { items?: { properties?: Record<string, unknown> } }).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<string, unknown> } }).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(
{
packages/coding-agent/test/task/task-progress-render.test.ts
+1 -54
View File
@@ -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> = {}): 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 = {