Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 45 additions & 4 deletions extensions/post-edit/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,18 @@ type Handler = (event: unknown, ctx: ExtensionContext) => unknown;

function deferred<T>() {
let resolve!: (value: T) => void;
const promise = new Promise<T>((done) => {
let reject!: (error: unknown) => void;
const promise = new Promise<T>((done, fail) => {
resolve = done;
reject = fail;
});
return { promise, resolve };
return { promise, resolve, reject };
}

function harness(mode: ExtensionContext["mode"] = "tui") {
function harness(
mode: ExtensionContext["mode"] = "tui",
command = "npm run format",
) {
const handlers = new Map<string, Handler[]>();
const executions: Array<{
command: string;
Expand Down Expand Up @@ -48,7 +53,7 @@ function harness(mode: ExtensionContext["mode"] = "tui") {
return result.promise;
},
} as unknown as ExtensionAPI;
postEdit(pi, () => "npm run format");
postEdit(pi, () => command);

const emit = async (event: string, value: unknown = {}) => {
for (const handler of handlers.get(event) ?? []) {
Expand Down Expand Up @@ -121,6 +126,42 @@ test("post-edit sanitizes failure notifications", async () => {
);
});

test("post-edit independently bounds the command and output in failure notifications", async () => {
const h = harness("tui", "x".repeat(500));
await h.emit("session_start");
await h.emit("tool_result", { toolName: "write", isError: false });
await h.emit("agent_settled");
h.executions[0]?.result.resolve({
stdout: "",
stderr: "y".repeat(1_000),
code: 127,
killed: false,
});
await new Promise((resolve) => setImmediate(resolve));

const notice = h.notifications[0] ?? "";
assert.match(notice, /exit 127/);
assert.match(notice, /x+…/);
assert.match(notice, /y+…/);
assert.ok([...notice].length < 600, "notification must stay compact");
assert.doesNotMatch(notice, /x{200}|y{400}/);
});

test("post-edit bounds execution errors before notifying", async () => {
const h = harness();
await h.emit("session_start");
await h.emit("tool_result", { toolName: "write", isError: false });
await h.emit("agent_settled");
h.executions[0]?.result.reject(new Error("z".repeat(1_000)));
await new Promise((resolve) => setImmediate(resolve));

const notice = h.notifications[0] ?? "";
assert.match(notice, /could not run/);
assert.match(notice, /z+…/);
assert.ok([...notice].length < 400, "execution error must stay compact");
assert.doesNotMatch(notice, /z{400}/);
});

test("post-edit aborts an in-flight command on session shutdown", async () => {
const h = harness();
await h.emit("session_start");
Expand Down
22 changes: 16 additions & 6 deletions extensions/post-edit/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,14 @@ import { sanitizeTerminalText } from "../shared/terminal-text.ts";

/** Tools whose success means a file on disk changed. */
const MUTATING_TOOLS = new Set(["write", "edit"]);
const NOTICE_COMMAND_MAX_CHARS = 160;
const NOTICE_DETAIL_MAX_CHARS = 320;

function boundedNoticeText(value: string, maxChars: number) {
const chars = [...sanitizeTerminalText(value).trim()];
if (chars.length <= maxChars) return chars.join("");
return `${chars.slice(0, maxChars - 1).join("")}…`;
}

export default function postEdit(
pi: ExtensionAPI,
Expand Down Expand Up @@ -75,18 +83,20 @@ export default function postEdit(
})
.then((result) => {
if (result.code === 0) return;
const detail = sanitizeTerminalText(
const commandPreview = boundedNoticeText(ran, NOTICE_COMMAND_MAX_CHARS);
const detail = boundedNoticeText(
result.stderr || result.stdout || "",
)
.trim()
.slice(0, 500);
NOTICE_DETAIL_MAX_CHARS,
);
warn(
`post-edit command failed (exit ${result.code}): ${sanitizeTerminalText(ran)}${detail ? `\n${detail}` : ""}`,
`post-edit command failed (exit ${result.code}): ${commandPreview}${detail ? `\n${detail}` : ""}`,
);
})
.catch((error: unknown) => {
const detail = error instanceof Error ? error.message : String(error);
warn(`post-edit command could not run: ${detail}`);
warn(
`post-edit command could not run: ${boundedNoticeText(detail, NOTICE_DETAIL_MAX_CHARS)}`,
);
})
.finally(() => {
if (active?.controller === controller) active = undefined;
Expand Down
66 changes: 61 additions & 5 deletions extensions/setup/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import assert from "node:assert/strict";
import test from "node:test";
import { mkdtempSync, rmSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import test, { after } from "node:test";
import type {
ExtensionAPI,
ExtensionCommandContext,
Expand All @@ -10,20 +13,41 @@ import {
OPENPI_SETUP_EPISODE_CHANNEL,
type OpenPiSetupEpisodeState,
} from "../shared/setup-episode-state.ts";
import setupExtension, {
const setupAgentDir = mkdtempSync(join(tmpdir(), "openpi-setup-index-"));
process.env.PI_CODING_AGENT_DIR = setupAgentDir;

const {
default: setupExtension,
applySubagentRoleModelUpdates,
buildInteractiveSetupPrompt,
buildSetupSuccessText,
CONFIGURE_MY_PI_SETUP_TOOL_NAME,
shouldOfferPiIntercom,
SUBAGENT_ROLE_MODELS_SCHEMA,
} from "./index.ts";
} = await import("./index.ts");
const { SETUP_CONFIG_PATH, loadSetupConfig } = await import(
"../shared/setup-config.ts"
);

after(() => rmSync(setupAgentDir, { recursive: true, force: true }));

type Handler = (
event: Record<string, unknown>,
ctx: ExtensionContext,
) => unknown;

interface CapturedSetupTool {
readonly name: string;
readonly parameters: unknown;
readonly execute: (
toolCallId: string,
params: Record<string, unknown>,
signal: AbortSignal,
onUpdate: (update: unknown) => void,
ctx: ExtensionContext,
) => Promise<unknown>;
}

function visibilityHarness(
options: {
initialActive?: string[];
Expand All @@ -35,7 +59,7 @@ function visibilityHarness(
string,
{ handler: (args: string, ctx: ExtensionCommandContext) => Promise<void> }
>();
const tools = new Map<string, { name: string; parameters: unknown }>();
const tools = new Map<string, CapturedSetupTool>();
const handlers = new Map<string, Handler[]>();
let activeTools = [
...(options.initialActive ?? ["read", "bash", "edit", "write"]),
Expand All @@ -61,7 +85,7 @@ function visibilityHarness(
) {
commands.set(name, command);
},
registerTool(tool: { name: string; parameters: unknown }) {
registerTool(tool: CapturedSetupTool) {
tools.set(tool.name, tool);
// Pi refreshTools() adds newly registered names to the active set.
if (!activeTools.includes(tool.name)) {
Expand Down Expand Up @@ -160,6 +184,11 @@ test("registers the canonical setup command, legacy alias, and one constrained t
assert.equal("suggestions_enabled" in parameters.properties, true);
assert.equal("suggestion_model" in parameters.properties, true);
assert.equal("capability_discovery" in parameters.properties, true);
const postEdit = parameters.properties.post_edit_command as {
description?: string;
};
assert.match(postEdit.description ?? "", /only when.*explicitly asks/i);
assert.match(postEdit.description ?? "", /omit to preserve/i);
assert.equal(
Object.keys(parameters.properties).some((name) =>
name.startsWith("summary"),
Expand All @@ -168,6 +197,33 @@ test("registers the canonical setup command, legacy alias, and one constrained t
);
});

test("post-edit stays off or preserved unless the setup request changes it", async () => {
rmSync(SETUP_CONFIG_PATH, { force: true });
const h = visibilityHarness();
const tool = h.tools.get(CONFIGURE_MY_PI_SETUP_TOOL_NAME);
assert.ok(tool);
const apply = (params: Record<string, unknown>) =>
tool.execute(
"setup-call",
params,
new AbortController().signal,
() => {},
h.ctx,
);

await apply({ ui_show_header: true });
assert.equal(loadSetupConfig().postEdit.command, "");

await apply({ post_edit_command: " npm run format " });
assert.equal(loadSetupConfig().postEdit.command, "npm run format");

await apply({ workflow_concurrency: 4 });
assert.equal(loadSetupConfig().postEdit.command, "npm run format");

await apply({ post_edit_command: "" });
assert.equal(loadSetupConfig().postEdit.command, "");
});

test("session_start hides configure_my_pi_setup after registration refresh", async () => {
const h = visibilityHarness();
assert.equal(h.isActive(), true, "registerTool refresh activates the tool");
Expand Down
2 changes: 1 addition & 1 deletion extensions/setup/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ export default function openPiSetup(pi: ExtensionAPI) {
Type.String({
maxLength: POST_EDIT_COMMAND_MAX_CHARS,
description:
'A single shell command (maximum 500 characters) to run in the background after a turn with successful Write/Edit operations, e.g. "npm run format". Runs once per changed turn, not per edit, and only in an interactive TUI session. Set to an empty string to turn it off. Omit to preserve the current value.',
'A single shell command (maximum 500 characters) to run in the background after a turn with successful Write/Edit operations, e.g. "npm run format". Set a non-empty command only when the current user\'s /openpi-setup request explicitly asks to configure Post-edit; do not infer one while changing another setting. Runs once per changed turn, not per edit, and only in an interactive TUI session. Set to an empty string to turn it off. Omit to preserve the current value.',
}),
),
}),
Expand Down
Loading