diff --git a/extensions/post-edit/index.test.ts b/extensions/post-edit/index.test.ts index d0dbb037..4b0c8037 100644 --- a/extensions/post-edit/index.test.ts +++ b/extensions/post-edit/index.test.ts @@ -12,13 +12,18 @@ type Handler = (event: unknown, ctx: ExtensionContext) => unknown; function deferred() { let resolve!: (value: T) => void; - const promise = new Promise((done) => { + let reject!: (error: unknown) => void; + const promise = new Promise((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(); const executions: Array<{ command: string; @@ -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) ?? []) { @@ -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"); diff --git a/extensions/post-edit/index.ts b/extensions/post-edit/index.ts index 3eb80b77..534d442f 100644 --- a/extensions/post-edit/index.ts +++ b/extensions/post-edit/index.ts @@ -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, @@ -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; diff --git a/extensions/setup/index.test.ts b/extensions/setup/index.test.ts index d517ef6f..b246c23c 100644 --- a/extensions/setup/index.test.ts +++ b/extensions/setup/index.test.ts @@ -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, @@ -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, ctx: ExtensionContext, ) => unknown; +interface CapturedSetupTool { + readonly name: string; + readonly parameters: unknown; + readonly execute: ( + toolCallId: string, + params: Record, + signal: AbortSignal, + onUpdate: (update: unknown) => void, + ctx: ExtensionContext, + ) => Promise; +} + function visibilityHarness( options: { initialActive?: string[]; @@ -35,7 +59,7 @@ function visibilityHarness( string, { handler: (args: string, ctx: ExtensionCommandContext) => Promise } >(); - const tools = new Map(); + const tools = new Map(); const handlers = new Map(); let activeTools = [ ...(options.initialActive ?? ["read", "bash", "edit", "write"]), @@ -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)) { @@ -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"), @@ -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) => + 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"); diff --git a/extensions/setup/index.ts b/extensions/setup/index.ts index 3c02bed7..3f97d298 100644 --- a/extensions/setup/index.ts +++ b/extensions/setup/index.ts @@ -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.', }), ), }),