From a5c8839acc3007594899467c14aaba6995bbdc3b Mon Sep 17 00:00:00 2001 From: youwang <61931019+MelodyVAR@users.noreply.github.com> Date: Thu, 1 Oct 2026 17:58:04 +0800 Subject: [PATCH] feat: add then_run verification to edit_file and write_file An optional then_run command runs after a successful mutation and its output and exit status are appended to the same tool result. It is gated as an embedded run_command call across permissions, workflow ACLs, active tools, and SDK acceptEdits. --- .../coding-agent/docs/step-integration.md | 21 ++ packages/coding-agent/src/features/step.ts | 10 + .../src/features/workflow/tool-profile.ts | 7 +- packages/coding-agent/src/step/permissions.ts | 36 ++++ packages/coding-agent/src/step/stdio-host.ts | 5 +- .../coding-agent/src/step/system-prompt.ts | 6 + packages/coding-agent/src/step/then-run.ts | 17 ++ .../coding-agent/src/step/tool-profile.ts | 182 ++++++++++++++++-- .../test/step-permissions.test.ts | 27 +++ .../test/step-system-prompt.test.ts | 2 + .../test/step-tool-profile.test.ts | 79 +++++++- .../test/workflow-runtime.test.ts | 8 + 12 files changed, 385 insertions(+), 15 deletions(-) create mode 100644 packages/coding-agent/src/step/then-run.ts diff --git a/packages/coding-agent/docs/step-integration.md b/packages/coding-agent/docs/step-integration.md index 99a396c7..db875407 100644 --- a/packages/coding-agent/docs/step-integration.md +++ b/packages/coding-agent/docs/step-integration.md @@ -234,6 +234,27 @@ editor refocus/disposal callback is cancelled before mounting. This prevents an orphaned promise, overlay, or timeout. Ordinary dismissal still allows the refocused editor to open the next dialog. +### Fused verification (`then_run`) + +`edit_file` and `write_file` accept an optional `then_run` shell command. It +runs only after the mutation succeeds (including a no-op), from the initial +working directory like `run_command` without `cwd`, with the same timeout and +output cap. Its output and exit status are appended to the mutation receipt +and recorded in `details.thenRun`. A failing check does not mark the call as +an error, because the file change has already been applied. Empty or +whitespace-only values are ignored. + +The Step extension gates `then_run` as an embedded `run_command` call: the +permission decision is the stricter of the mutation and the command +(including `run_command` overrides and dangerous-command analysis), the +confirmation reason names the command, workflow path ACLs check it, and the +call is blocked when `run_command` is not active in the session. SDK +`acceptEdits` does not auto-approve an edit carrying `then_run`. SDK +`PreToolUse` hooks and third-party `tool_call` handlers see it as +`input.then_run`; embedders that skip the Step extension must gate it +themselves. In plan mode a `then_run` on the plan file behaves like +`run_command`, which keeps normal permissions there. + ## Plans and tasks Plans and tasks serve different purposes. A **plan** is the Markdown proposal: diff --git a/packages/coding-agent/src/features/step.ts b/packages/coding-agent/src/features/step.ts index bc8c1347..72f9568c 100644 --- a/packages/coding-agent/src/features/step.ts +++ b/packages/coding-agent/src/features/step.ts @@ -24,6 +24,7 @@ import type { StepSettingsManager } from "../step/settings-manager.ts"; import { recordStepSlashCommand, registerStepPiCommandAdapters } from "../step/slash-commands.ts"; import { type StepTelemetryReporter, trackStepTelemetry } from "../step/telemetry.ts"; import type { TraceHeaderPolicy } from "../step/telemetry-contract.ts"; +import { getThenRunCommand } from "../step/then-run.ts"; import { applyStepTraceHeaders } from "../step/trace-headers.ts"; import { fetchStepModelEfforts, @@ -243,6 +244,15 @@ export function createStepExtension(options: StepExtensionOptions = {}): Extensi }); pi.on("tool_call", async (event, ctx) => { + if ( + getThenRunCommand(event.toolName, event.input) !== undefined && + !pi.getActiveTools().includes("run_command") + ) { + return { + block: true, + reason: "then_run requires run_command, which is not available in this session; retry without then_run", + }; + } return await permissions.handleToolCall(event, ctx); }); diff --git a/packages/coding-agent/src/features/workflow/tool-profile.ts b/packages/coding-agent/src/features/workflow/tool-profile.ts index 45693e5b..5264a6d5 100644 --- a/packages/coding-agent/src/features/workflow/tool-profile.ts +++ b/packages/coding-agent/src/features/workflow/tool-profile.ts @@ -1,5 +1,6 @@ import { existsSync, realpathSync } from "node:fs"; import path from "node:path"; +import { getThenRunCommand } from "../../step/then-run.ts"; const READ_ONLY_TOOLS = ["read_file", "search_files", "find_files", "list_directory", "find_tools"]; const DEVELOPER_TOOLS = [...READ_ONLY_TOOLS, "write_file", "edit_file", "run_command"]; @@ -156,7 +157,11 @@ export function checkWorkflowToolCall( const target = ["path", "filePath", "target", "filename"] .map((key) => value[key]) .find((item) => typeof item === "string"); - return checkWorkflowPathAccess(cwd, typeof target === "string" ? target : "", "write", acl); + const writeDecision = checkWorkflowPathAccess(cwd, typeof target === "string" ? target : "", "write", acl); + const thenRun = getThenRunCommand(toolName, value); + if (!writeDecision.allowed || thenRun === undefined) return writeDecision; + const runDecision = checkWorkflowToolCall(cwd, "run_command", { command: thenRun }, acl); + return runDecision.allowed ? writeDecision : runDecision; } if (EXECUTE_TOOL_NAMES.has(toolName)) { const commandCwd = typeof value.cwd === "string" ? value.cwd : "."; diff --git a/packages/coding-agent/src/step/permissions.ts b/packages/coding-agent/src/step/permissions.ts index c4ce647d..ea1dc11d 100644 --- a/packages/coding-agent/src/step/permissions.ts +++ b/packages/coding-agent/src/step/permissions.ts @@ -17,6 +17,7 @@ import type { import { getShellConfig } from "../utils/shell.ts"; import { analyzeCommandPolicy, type CommandPolicyAnalysis } from "./command-policy.ts"; +import { getThenRunCommand } from "./then-run.ts"; export { containsDangerousLifecycleCommand, isDangerousCommand } from "./command-policy.ts"; @@ -333,6 +334,34 @@ export function resolveInitialStepPermissionState(options: StepPermissionControl return resolved; } +const DECISION_RANK = { allow: 0, confirm: 1, deny: 2 } as const; + +/** + * A then_run command is an embedded run_command call: the fused call takes the + * stricter of the two decisions, and a confirmation names the command because + * the dialog's input summary may clip it behind a large file payload. + */ +function combineThenRunDecisions( + mutation: StepToolDecision, + verification: StepToolDecision, + command: string, +): StepToolDecision { + const flagged = (decision: StepToolDecision) => decision.hazardous || decision.analysisIncomplete === true; + const verificationRank = DECISION_RANK[verification.action]; + const mutationRank = DECISION_RANK[mutation.action]; + const verificationLeads = + verificationRank > mutationRank || + (verificationRank === mutationRank && flagged(verification) && !flagged(mutation)); + const lead = verificationLeads ? verification : mutation; + const baseReason = verificationLeads ? `then_run (run_command): ${verification.reason}` : mutation.reason; + return { + action: lead.action, + hazardous: mutation.hazardous || verification.hazardous, + ...(mutation.analysisIncomplete || verification.analysisIncomplete ? { analysisIncomplete: true as const } : {}), + reason: lead.action === "confirm" ? `${baseReason}\nthen_run: ${command}` : baseReason, + }; +} + /** * Decide a tool call without involving the terminal. This is intentionally * conservative for unknown tools: ask mode confirms them, read-only blocks @@ -345,6 +374,13 @@ export function decideStepToolCall( overrides: Readonly> | undefined = state.toolOverrides, shellContext?: ShellExecutionContext, ): StepToolDecision { + const thenRun = getThenRunCommand(toolName, input); + if (thenRun !== undefined) { + const { then_run: _thenRun, ...mutationInput } = input; + const mutation = decideStepToolCall(toolName, mutationInput, state, overrides, shellContext); + const verification = decideStepToolCall("run_command", { command: thenRun }, state, overrides, shellContext); + return combineThenRunDecisions(mutation, verification, thenRun); + } const normalizedName = toolName.trim().toLowerCase(); const command = extractCommand(input); let analysis: CommandPolicyAnalysis | undefined; diff --git a/packages/coding-agent/src/step/stdio-host.ts b/packages/coding-agent/src/step/stdio-host.ts index 410d0bd9..92a7bb7a 100644 --- a/packages/coding-agent/src/step/stdio-host.ts +++ b/packages/coding-agent/src/step/stdio-host.ts @@ -35,6 +35,7 @@ import { StepStdioFrameDecoder, StepStdioProtocolViolation, } from "./stdio.ts"; +import { getThenRunCommand } from "./then-run.ts"; type FrameWriter = (chunk: Buffer) => boolean | undefined; @@ -798,7 +799,9 @@ export class StepStdioHost { if (mode === "plan" || mode === "dontAsk") { return { block: true, reason: `tool ${toolName} is not allowed in permission mode ${mode}`, terminate: true }; } - if (mode === "acceptEdits" && isEditTool(toolName)) return undefined; + if (mode === "acceptEdits" && isEditTool(toolName) && getThenRunCommand(toolName, context.args) === undefined) { + return undefined; + } if (query.options.hasPermissionCallback !== true) { return { block: true, diff --git a/packages/coding-agent/src/step/system-prompt.ts b/packages/coding-agent/src/step/system-prompt.ts index 321365a6..902c3b31 100644 --- a/packages/coding-agent/src/step/system-prompt.ts +++ b/packages/coding-agent/src/step/system-prompt.ts @@ -364,6 +364,12 @@ export function buildStepSystemPromptAppendix( "- Keep tool calls narrow and independently verifiable. Do not use interactive commands or shell chains when a structured argument (such as cwd) is available.", ]; + if ((active.has("edit_file") || active.has("write_file")) && active.has("run_command")) { + sections.push( + "- To verify a change immediately, pass the check as then_run on edit_file or write_file (for example a focused test or typecheck) instead of a separate run_command call; it runs only after the change succeeds and needs the same approval as run_command.", + ); + } + if (hasWrite || hasExecute) { sections.push( "For large source files and reports, create a small initial section, then grow it with focused edits across separate responses. Keep generated code or text in tool arguments to roughly 100 lines or a few kilobytes per response when practical; this is a planning guideline, not permission to truncate content. Do not combine many large writes in one response or embed the same large payload in a shell command. Complete all sections before final validation and report any unfinished work.", diff --git a/packages/coding-agent/src/step/then-run.ts b/packages/coding-agent/src/step/then-run.ts new file mode 100644 index 00000000..72f05380 --- /dev/null +++ b/packages/coding-agent/src/step/then-run.ts @@ -0,0 +1,17 @@ +/** + * Action fusion: edit_file/write_file accept an optional `then_run` command + * that runs after the mutation succeeds. Gates (permissions, workflow ACL, + * active tools, SDK acceptEdits) treat it as an embedded run_command call. + */ + +export const THEN_RUN_TOOL_NAMES: ReadonlySet = new Set(["edit_file", "write_file"]); + +export function readThenRunCommand(input: unknown): string | undefined { + if (!input || typeof input !== "object" || Array.isArray(input)) return undefined; + const value = (input as Record).then_run; + return typeof value === "string" && value.trim().length > 0 ? value : undefined; +} + +export function getThenRunCommand(toolName: string, input: unknown): string | undefined { + return THEN_RUN_TOOL_NAMES.has(toolName.trim().toLowerCase()) ? readThenRunCommand(input) : undefined; +} diff --git a/packages/coding-agent/src/step/tool-profile.ts b/packages/coding-agent/src/step/tool-profile.ts index 708f514b..d0de6a02 100644 --- a/packages/coding-agent/src/step/tool-profile.ts +++ b/packages/coding-agent/src/step/tool-profile.ts @@ -14,7 +14,7 @@ import { mkdir as fsMkdir, readdir as fsReaddir, stat as fsStat, readFile, write import os from "node:os"; import path from "node:path"; import type { AgentToolResult } from "@step-harness/agent-core"; -import type { Component } from "@step-harness/pi-tui"; +import { type Component, Text } from "@step-harness/pi-tui"; import { type Static, Type } from "typebox"; import type { AgentToolUpdateCallback, @@ -57,6 +57,7 @@ import { } from "../utils/shell.ts"; import { resolveStepAgentDir } from "./environment.ts"; import { createSearchWebTool, type SearchWebToolOptions } from "./search-web-tool.ts"; +import { readThenRunCommand } from "./then-run.ts"; const STEP_TOOL_NAMES = [ "list_directory", @@ -83,6 +84,8 @@ const READ_FILE_DESCRIPTION = "Read a text file with optional line range; image files (PNG/JPEG/GIF/WebP) are returned as attached images. Prefer this over shell cat for token efficiency."; const WRITE_FILE_DESCRIPTION = "Write full content to a file, creating parent directories if missing. Overwrites existing content — for existing files prefer edit_file."; +const THEN_RUN_DESCRIPTION = + "Optional shell command to run after the change succeeds, from the initial working directory like run_command (e.g. a focused test or typecheck); its output and exit status are appended to this result, saving a separate run_command call. Requires the same approval as run_command."; const listDirectorySchema = Type.Object({ path: Type.Optional(Type.String({ description: `Directory path. ${PATH_DESCRIPTION} Defaults to '.'` })), @@ -118,6 +121,7 @@ const readFileSchema = Type.Object({ const writeFileSchema = Type.Object({ path: Type.String({ description: `File path. ${PATH_DESCRIPTION}` }), content: Type.String({ description: "Full file content" }), + then_run: Type.Optional(Type.String({ description: THEN_RUN_DESCRIPTION })), }); const editFileSchema = Type.Object({ @@ -125,6 +129,7 @@ const editFileSchema = Type.Object({ search: Type.String({ description: "Literal string to find" }), replace: Type.String({ description: "Replacement string" }), replace_all: Type.Optional(Type.Boolean({ description: "Replace all matches" })), + then_run: Type.Optional(Type.String({ description: THEN_RUN_DESCRIPTION })), }); const runCommandSchema = Type.Object({ @@ -1019,6 +1024,149 @@ function mapRunCommandArgs(args: RunCommandInput, ctx?: ExtensionContext): { com }; } +export interface StepThenRunDetails { + command: string; + exitCode: number | null; + timedOut: boolean; + stepTruncated: boolean; +} + +/** + * Action fusion: run the `then_run` verification command after a successful + * file mutation and fold its outcome into the mutation result. The native + * bash tool throws on non-zero exits; a failing check must not hide the + * already-applied mutation, so those throws become part of the fused output + * and the result is not marked as an error. Caller cancellation still + * propagates, carrying the mutation receipt. + */ +async function withThenRunResult( + result: AnyResult, + nativeBash: AnyToolDefinition, + command: string, + signal: AbortSignal | undefined, + ctx: ExtensionContext | undefined, +): Promise { + const configuredTimeout = getContextValue(ctx, "commandTimeoutMs"); + const receipt = textFromResult(result); + let output: string; + let status: string | undefined; + let exitCode: number | null = 0; + let timedOut = false; + try { + // No onUpdate: bash partials would reach the edit/write renderers. + const bashResult = await nativeBash.execute( + "step-then-run", + { command, timeout: typeof configuredTimeout === "number" ? configuredTimeout / 1000 : undefined }, + signal, + undefined, + ctx as ExtensionContext, + ); + output = textFromResult(bashResult); + status = "Command exited with code 0"; + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + if (isCallerAborted(signal)) throw new Error(`${receipt}\n\nthen_run: $ ${command}\n${message}`); + const match = /(?:^|\n\n)(Command exited with code (\d+)|Command timed out after .+ seconds)$/u.exec(message); + if (match) { + output = message.slice(0, match.index); + status = match[1]; + exitCode = match[2] === undefined ? null : Number(match[2]); + timedOut = match[2] === undefined; + } else { + output = message; + exitCode = null; + } + } + const limited = withStepTextLimit("run_command", output || "(no output)", resolveStepMaxChars(undefined, ctx)); + const thenRun: StepThenRunDetails = { command, exitCode, timedOut, stepTruncated: limited.truncated }; + return { + ...result, + content: [ + { + type: "text", + text: `${receipt}\n\n${thenRunMarker(command)}${limited.text}${status ? `\n\n${status}` : ""}`, + }, + ], + details: { ...nativeDetails(result), thenRun }, + } as AnyResult; +} + +function thenRunMarker(command: string): string { + return `then_run: $ ${command}\n`; +} + +function readThenRunDetails(result: AnyResult): StepThenRunDetails | undefined { + const value = nativeDetails(result).thenRun; + return value && typeof value === "object" && typeof (value as StepThenRunDetails).command === "string" + ? (value as StepThenRunDetails) + : undefined; +} + +/** Appends the then_run summary below the native edit/write result component. */ +class ThenRunResultComponent implements Component { + readonly wantsKeyRelease?: boolean; + readonly inner: Component; + readonly extra: Text; + private cache?: { innerLines: string[]; extraLines: string[]; lines: string[] }; + + constructor(inner: Component, text: string) { + this.inner = inner; + this.extra = new Text(text, 0, 0); + this.wantsKeyRelease = inner.wantsKeyRelease; + } + + render(width: number): string[] { + const innerLines = this.inner.render(width); + const extraLines = this.extra.render(width); + // Return a stable array so parent containers can reuse their cached prefix. + if (this.cache?.innerLines === innerLines && this.cache.extraLines === extraLines) return this.cache.lines; + const lines = [...innerLines, ...extraLines]; + this.cache = { innerLines, extraLines, lines }; + return lines; + } + + handleInput(data: string): void { + this.inner.handleInput?.(data); + } + + invalidate(): void { + this.cache = undefined; + this.inner.invalidate(); + this.extra.invalidate(); + } +} + +function formatThenRunSummary(result: AnyResult, thenRun: StepThenRunDetails, expanded: boolean, theme: any): string { + const command = thenRun.command.replace(/\s+/gu, " ").trim(); + const status = thenRun.timedOut + ? theme.fg("error", "timed out") + : thenRun.exitCode === 0 + ? theme.fg("success", "exit 0") + : theme.fg("error", thenRun.exitCode === null ? "failed" : `exit ${thenRun.exitCode}`); + const summary = `${theme.fg("muted", `then_run $ ${command} ·`)} ${status}`; + if (!expanded) return summary; + const text = textFromResult(result); + const markerIndex = text.indexOf(thenRunMarker(thenRun.command)); + if (markerIndex < 0) return summary; + const body = text + .slice(markerIndex + thenRunMarker(thenRun.command).length) + .replace(/\n\n(Command exited with code \d+|Command timed out after .+ seconds)$/u, ""); + return `${theme.fg("toolOutput", body)}\n${summary}`; +} + +/** Wrap a native edit/write result renderer so then_run output stays visible. */ +function withThenRunRenderResult(renderResult: Renderer["renderResult"]): Renderer["renderResult"] { + if (!renderResult) return undefined; + return (result, options, theme, context) => { + const lastComponent = + context.lastComponent instanceof ThenRunResultComponent ? context.lastComponent.inner : context.lastComponent; + const inner = renderResult(result, options, theme, { ...context, lastComponent }); + const thenRun = options.isPartial ? undefined : readThenRunDetails(result); + if (!thenRun) return inner; + return new ThenRunResultComponent(inner, formatThenRunSummary(result, thenRun, options.expanded, theme)); + }; +} + /** Execute Step's literal edit contract while retaining Pi's renderer/preview. */ async function executeStepEdit( native: ToolDefinition, @@ -1351,13 +1499,18 @@ export function createStepToolProfile(cwd: string, options: StepToolProfileOptio ); const writeFile = { ...writeFileBase, - execute: ( + execute: async ( _toolCallId: string, args: WriteFileInput, signal: AbortSignal | undefined, _onUpdate: AgentToolUpdateCallback | undefined, ctx: ExtensionContext, - ) => executeStepWriteFile(args, cwd, options.write, signal, ctx), + ) => { + const result = await executeStepWriteFile(args, cwd, options.write, signal, ctx); + const thenRun = readThenRunCommand(args); + return thenRun === undefined ? result : withThenRunResult(result, nativeBash, thenRun, signal, ctx); + }, + renderResult: withThenRunRenderResult(writeFileBase.renderResult as Renderer["renderResult"]), } as AnyToolDefinition; const editFile: ToolDefinition = { @@ -1373,8 +1526,11 @@ export function createStepToolProfile(cwd: string, options: StepToolProfileOptio constrainedSampling: nativeEdit.constrainedSampling, executionMode: nativeEdit.executionMode, renderShell: nativeEdit.renderShell, - execute: (toolCallId, args, signal, onUpdate, ctx) => - executeStepEdit(nativeEdit, toolCallId, args, cwd, options.edit, signal, onUpdate, ctx), + execute: async (toolCallId, args, signal, onUpdate, ctx) => { + const result = await executeStepEdit(nativeEdit, toolCallId, args, cwd, options.edit, signal, onUpdate, ctx); + const thenRun = readThenRunCommand(args); + return thenRun === undefined ? result : withThenRunResult(result, nativeBash, thenRun, signal, ctx); + }, renderCall: nativeEdit.renderCall ? (args, theme, context) => nativeEdit.renderCall!(mapEditFileArgs(args), theme, { @@ -1382,13 +1538,15 @@ export function createStepToolProfile(cwd: string, options: StepToolProfileOptio args: mapEditFileArgs(args), }) : undefined, - renderResult: nativeEdit.renderResult - ? (result, renderOptions, theme, context) => - nativeEdit.renderResult!(result, renderOptions, theme, { - ...context, - args: mapEditFileArgs(context.args), - }) - : undefined, + renderResult: withThenRunRenderResult( + nativeEdit.renderResult + ? (result, renderOptions, theme, context) => + nativeEdit.renderResult!(result, renderOptions, theme, { + ...context, + args: mapEditFileArgs(context.args), + }) + : undefined, + ), }; const runCommand: ToolDefinition = { diff --git a/packages/coding-agent/test/step-permissions.test.ts b/packages/coding-agent/test/step-permissions.test.ts index 93a74a6b..10565500 100644 --- a/packages/coding-agent/test/step-permissions.test.ts +++ b/packages/coding-agent/test/step-permissions.test.ts @@ -108,6 +108,14 @@ describe("Step permission presets", () => { expect(decision.action).toBe("confirm"); expect(decision.hazardous).toBe(true); } + const fused = decideStepToolCall( + "write_file", + { path: "a.txt", content: "x", then_run: "rm -rf /etc" }, + stepPermissionStateForPreset("bypass"), + ); + expect(fused.action).toBe("confirm"); + expect(fused.hazardous).toBe(true); + expect(fused.reason).toContain("then_run: rm -rf /etc"); expect(isDangerousCommand("sudo -n reboot")).toBe(true); expect(isDangerousCommand("qemu-system-x86_64 -no-reboot -no-shutdown")).toBe(false); expect(containsDangerousLifecycleCommand("sh -c 'systemctl reboot'")).toBe(true); @@ -608,4 +616,23 @@ describe("Step autopilot continuation", () => { await Promise.resolve(); expect(announce).toHaveBeenCalledWith("Autopilot could not resume: session closed"); }); + + it("gates edit_file then_run as an embedded run_command call", () => { + const bypass = stepPermissionStateForPreset("bypass"); + const input = { path: "a.txt", search: "a", replace: "b", then_run: "npm test" }; + expect(decideStepToolCall("edit_file", input, bypass).action).toBe("allow"); + + const denied = decideStepToolCall("edit_file", input, bypass, { run_command: "deny" }); + expect(denied.action).toBe("deny"); + expect(denied.reason).toContain("then_run (run_command)"); + + const ask = decideStepToolCall("edit_file", input, stepPermissionStateForPreset("ask")); + expect(ask.action).toBe("confirm"); + expect(ask.reason).toContain("then_run: npm test"); + + const readOnly = stepPermissionStateForPreset("read-only"); + expect(decideStepToolCall("edit_file", input, readOnly)).toEqual( + decideStepToolCall("edit_file", { path: "a.txt", search: "a", replace: "b" }, readOnly), + ); + }); }); diff --git a/packages/coding-agent/test/step-system-prompt.test.ts b/packages/coding-agent/test/step-system-prompt.test.ts index 336fe4c0..d50a11a9 100644 --- a/packages/coding-agent/test/step-system-prompt.test.ts +++ b/packages/coding-agent/test/step-system-prompt.test.ts @@ -23,6 +23,8 @@ describe("Step system prompt appendix", () => { expect(prompt).toContain("- edit_file:"); expect(prompt).toContain("- run_command:"); expect(prompt).not.toContain("- write_file:"); + expect(prompt).toContain("then_run on edit_file or write_file"); + expect(buildStepSystemPromptAppendix(["read_file", "edit_file"])).not.toContain("then_run"); }); it("returns a useful base contract when no tools are active", () => { diff --git a/packages/coding-agent/test/step-tool-profile.test.ts b/packages/coding-agent/test/step-tool-profile.test.ts index 84b4f08e..22b5504c 100644 --- a/packages/coding-agent/test/step-tool-profile.test.ts +++ b/packages/coding-agent/test/step-tool-profile.test.ts @@ -30,7 +30,7 @@ describe("Step tool profile", () => { const edit = tools.find((tool) => tool.name === "edit_file")!; const editSchema = edit.parameters as { required?: string[]; properties: Record }; expect(editSchema.required).toEqual(["path", "search", "replace"]); - expect(Object.keys(editSchema.properties)).toEqual(["path", "search", "replace", "replace_all"]); + expect(Object.keys(editSchema.properties)).toEqual(["path", "search", "replace", "replace_all", "then_run"]); }); it("keeps the legacy model-facing descriptions byte-for-byte compatible", () => { @@ -499,4 +499,81 @@ describe("Step tool profile", () => { ); expect(text(webResult)).toContain("search_web"); }); + + it("then_run appends the verification outcome to edit_file and write_file results", async () => { + const root = await mkdtemp(join(tmpdir(), "step-tool-profile-")); + try { + const tools = createStepToolProfile(root); + const edit = tools.find((tool) => tool.name === "edit_file")!; + const write = tools.find((tool) => tool.name === "write_file")!; + await writeFile(join(root, "sample.txt"), "alpha\n"); + + const passed = await edit.execute( + "then-run-pass", + { path: "sample.txt", search: "alpha", replace: "beta", then_run: "cat sample.txt" }, + undefined, + undefined, + undefined as never, + ); + expect(text(passed)).toContain("Successfully replaced 1 occurrence(s) in sample.txt."); + expect(text(passed)).toContain("then_run: $ cat sample.txt\nbeta"); + expect(text(passed)).toMatch(/Command exited with code 0$/u); + expect((passed.details as { thenRun: unknown }).thenRun).toEqual({ + command: "cat sample.txt", + exitCode: 0, + timedOut: false, + stepTruncated: false, + }); + + const failed = await edit.execute( + "then-run-fail", + { path: "sample.txt", search: "beta", replace: "gamma", then_run: "echo broken; exit 3" }, + undefined, + undefined, + undefined as never, + ); + expect(await readFile(join(root, "sample.txt"), "utf8")).toBe("gamma\n"); + expect(text(failed)).toContain("broken"); + expect(text(failed)).toMatch(/Command exited with code 3$/u); + expect((failed.details as { thenRun: { exitCode: number } }).thenRun.exitCode).toBe(3); + + const written = await write.execute( + "then-run-write", + { path: "new.txt", content: "fresh\n", then_run: "cat new.txt" }, + undefined, + undefined, + undefined as never, + ); + expect(text(written)).toContain("Wrote 6 chars to new.txt."); + expect(text(written)).toContain("then_run: $ cat new.txt\nfresh"); + + const noop = await write.execute( + "then-run-noop", + { path: "new.txt", content: "fresh\n", then_run: "echo checked" }, + undefined, + undefined, + undefined as never, + ); + expect(text(noop)).toContain("already matches"); + expect(text(noop)).toContain("checked"); + + for (const thenRun of [undefined, " "]) { + const plain = await write.execute( + "then-run-skip", + { + path: "plain.txt", + content: `${thenRun ?? "x"}\n`, + ...(thenRun === undefined ? {} : { then_run: thenRun }), + }, + undefined, + undefined, + undefined as never, + ); + expect(text(plain)).not.toContain("then_run"); + expect((plain.details as { thenRun?: unknown }).thenRun).toBeUndefined(); + } + } finally { + await rm(root, { recursive: true, force: true }); + } + }); }); diff --git a/packages/coding-agent/test/workflow-runtime.test.ts b/packages/coding-agent/test/workflow-runtime.test.ts index 68b04904..da55614f 100644 --- a/packages/coding-agent/test/workflow-runtime.test.ts +++ b/packages/coding-agent/test/workflow-runtime.test.ts @@ -424,6 +424,14 @@ test("path ACL blocks read-only writes, traversal, and symlink escapes", async ( expect(checkWorkflowToolCall(cwd, "write_file", { path: path.join(cwd, "output", "ok.ts") }, acl).allowed).toBe( true, ); + expect( + checkWorkflowToolCall( + cwd, + "write_file", + { path: path.join(cwd, "output", "ok.ts"), then_run: `echo changed > '${artifact}/file.ts'` }, + acl, + ).allowed, + ).toBe(false); }); function plan(objective: string) {