Reviewed and proposed improvements for bash execution PR
Public
Reviewed and proposed improvements for bash execution PR
Agent
Pi
Model
Claude Opus 4.5 (latest)
Started
Jan 25, 6:34 PM
Messages58
Downloads0
Reviewed PR details, linked issues, and code changes.
UnderstandAnalyzed error handling and use cases.
UnderstandIdentified inconsistencies and design flaws.
UnderstandProposed a simplified, composable hook design.
Plan
You are given one or more GitHub PR URLs: https://github.com/badlogic/pi-mono/pull/903
For each PR URL, do the following in order:
packages/*/CHANGELOG.md files. Report whether an entry exists. If missing, state that a changelog entry is required before merge and that you will add it if the user decides to merge. Follow the changelog format rules in AGENTS.md. Verify:
### Breaking Changes, ### Added, ### Fixed, etc.)Fixed foo ([#123](https://github.com/badlogic/pi-mono/pull/123) by [@user](https://github.com/user))### Breaking Changes, not just ### FixedOutput format per PR: PR: Changelog:
If no issues are found, say so under Bad and Ugly.
>cd /Users/badlogic/workspaces/pi-mono && gh pr view 903 --json title,body,comments,commits,files,author,state,labels,headRefName,baseRefName{"author":{"id":"MDQ6VXNlcjczOTY=","is_bot":false,"login":"mitsuhiko","name":"Armin Ronacher"},"baseRefName":"main","body":"Discussion:\r\n\r\n<img width=\"1020\" height=\"350\" alt=\"image\" src=\"https://github.com/user-attachments/assets/a2a1633a-acee-40d8-a7ba-2d2ab9808514\" />\r\n\r\nThis allows hooking bash execution and it also allows `tool_result` hooks to manipulate error messages by \"abusing\" content to carry error messages. This feels somewhat consistent to how it works but a bit ugly.","comments":[{"id":"IC_kwDOPbFNk87huidp","author":{"login":"cv"},"authorAssociation":"CONTRIBUTOR","body":"Happy to modify/rewrite the examples/ssh.ts (and my own pi-ssh-remote) when this lands.","createdAt":"2026-01-22T22:32:17Z","includesCreatedEdit":false,"isMinimized":false,"minimizedReason":"","reactionGroups":[],"url":"https://github.com/badlogic/pi-mono/pull/903#issuecomment-3787073385","viewerDidAuthor":false},{"id":"IC_kwDOPbFNk87h1et-","author":{"login":"mitsuhiko"},"authorAssociation":"CONTRIBUTOR","body":"@cv it should in theory work with it already, the question in a way is if ssh.ts can further benefit from the changes.","createdAt":"2026-01-23T08:01:00Z","includesCreatedEdit":false,"isMinimized":false,"minimizedReason":"","reactionGroups":[],"url":"https://github.com/badlogic/pi-mono/pull/903#issuecomment-3788893054","viewerDidAuthor":false},{"id":"IC_kwDOPbFNk87h5XIx","author":{"login":"dejanr"},"authorAssociation":"NONE","body":"This could be really nice for project purposes, instead of blowing agents.md and instructions on how to run sandboxed environment, we could just build custom extensions that are preparing env for tool, is that nix develop, or uv, or whatever else that someone uses.\r\n\r\nFrom architectural point of view i would prefer a middleware approach to tool wrapping, then dangling handlers.","createdAt":"2026-01-23T11:56:52Z","includesCreatedEdit":true,"isMinimized":false,"minimizedReason":"","reactionGroups":[],"url":"https://github.com/badlogic/pi-mono/pull/903#issuecomment-3789910577","viewerDidAuthor":false},{"id":"IC_kwDOPbFNk87iPMHC","author":{"login":"mitsuhiko"},"authorAssociation":"CONTRIBUTOR","body":"@dejanr @Mic92 I think it would be nice if people other than me would test drive this and give some suggestions. It's quite narrowly scoped to what i tried so far.","createdAt":"2026-01-24T22:16:42Z","includesCreatedEdit":false,"isMinimized":false,"minimizedReason":"","reactionGroups":[],"url":"https://github.com/badlogic/pi-mono/pull/903#issuecomment-3795632578","viewerDidAuthor":false}],"commits":[{"authoredDate":"2026-01-22T18:47:03Z","authors":[{"email":"[REDACTED]","id":"MDQ6VXNlcjczOTY=","login":"mitsuhiko","name":"Armin Ronacher"}],"committedDate":"2026-01-22T18:47:03Z","messageBody":"","messageHeadline":"feat(coding-agent): intercept bash execution","oid":"15b91544b5b9045ccaa283c6eb136951231fb89f"},{"authoredDate":"2026-01-22T19:00:15Z","authors":[{"email":"[REDACTED]","id":"MDQ6VXNlcjczOTY=","login":"mitsuhiko","name":"Armin Ronacher"}],"committedDate":"2026-01-22T19:52:36Z","messageBody":"","messageHeadline":"Reuse and recycle","oid":"e25be49907f074dee15d00955de4010550af5190"},{"authoredDate":"2026-01-22T19:58:09Z","authors":[{"email":"[REDACTED]","id":"MDQ6VXNlcjczOTY=","login":"mitsuhiko","name":"Armin Ronacher"}],"committedDate":"2026-01-22T19:58:09Z","messageBody":"","messageHeadline":"Fix test","oid":"95c03063adf375c82f4b43a2a3c17a9338538c88"}],"files":[{"path":"packages/coding-agent/README.md","additions":17,"deletions":1},{"path":"packages/coding-agent/docs/extensions.md","additions":29,"deletions":2},{"path":"packages/coding-agent/examples/extensions/uv.ts","additions":74,"deletions":0},{"path":"packages/coding-agent/src/core/agent-session.ts","additions":47,"deletions":8},{"path":"packages/coding-agent/src/core/bash-executor.ts","additions":45,"deletions":6},{"path":"packages/coding-agent/src/core/extensions/index.ts","additions":5,"deletions":0},{"path":"packages/coding-agent/src/core/extensions/runner.ts","additions":76,"deletions":0},{"path":"packages/coding-agent/src/core/extensions/types.ts","additions":42,"deletions":0},{"path":"packages/coding-agent/src/core/extensions/wrapper.ts","additions":92,"deletions":9},{"path":"packages/coding-agent/src/core/messages.ts","additions":1,"deletions":0},{"path":"packages/coding-agent/src/core/tools/bash.ts","additions":29,"deletions":7},{"path":"packages/coding-agent/src/index.ts","additions":5,"deletions":0},{"path":"packages/coding-agent/src/utils/shell.ts","additions":29,"deletions":2},{"path":"packages/coding-agent/test/tools.test.ts","additions":4,"deletions":3}],"headRefName":"intercept-bash","labels":[],"state":"OPEN","title":"Add support for intercepting bash"}>cd /Users/badlogic/workspaces/pi-mono && gh pr diff 903diff --git a/packages/coding-agent/README.md b/packages/coding-agent/README.md
index 79d78c435..edb7f880a 100644
--- a/packages/coding-agent/README.md
+++ b/packages/coding-agent/README.md
@@ -1101,7 +1101,23 @@ export default function (pi: ExtensionAPI) {
pi.on("tool_result", async (event, ctx) => {
if (event.toolName === "read") {
// Redact secrets from file contents
- return { modifiedResult: event.result.replace(/API_KEY=[REDACTED] "API_KEY=[REDACTED] };
+ return {
+ content: event.content.map((item) =>
+ item.type === "text"
+ ? { ...item, text: item.text.replace(/API_KEY=[REDACTED] "API_KEY=[REDACTED] }
+ : item
+ ),
+ };
+ }
+
+ if (event.isError) {
+ // Override the thrown error message (optional)
+ return { content: [{ type: "text", text: "Custom error message" }], isError: true };
+ }
+
+ if (event.toolName === "bash" && event.content.length > 0) {
+ // Force a successful tool to be treated as an error
+ return { content: [{ type: "text", text: "Tool output rejected by policy" }], isError: true };
}
});
diff --git a/packages/coding-agent/docs/extensions.md b/packages/coding-agent/docs/extensions.md
index 5149ef95b..44aec04b3 100644
--- a/packages/coding-agent/docs/extensions.md
+++ b/packages/coding-agent/docs/extensions.md
@@ -549,9 +549,34 @@ pi.on("tool_call", async (event, ctx) => {
**Examples:** [chalk-logger.ts](../examples/extensions/chalk-logger.ts), [permission-gate.ts](../examples/extensions/permission-gate.ts), [plan-mode/index.ts](../examples/extensions/plan-mode/index.ts), [protected-paths.ts](../examples/extensions/protected-paths.ts)
+#### before_bash_exec
+
+Fired before a bash command executes (tool calls and user `!`/`!!`). Use it to rewrite commands or override execution settings. You can also block execution by returning `{ block: true, reason?: string }`. For follow-up hints based on output, pair this with `tool_result`.
+
+```typescript
+pi.on("before_bash_exec", async (event) => {
+ if (event.command.includes("rm -rf")) {
+ return { block: true, reason: "Blocked by policy" };
+ }
+
+ if (event.source === "tool") {
+ return {
+ cwd: "/tmp",
+ env: {
+ ...event.env,
+ MY_VAR: "1",
+ PATH: undefined, // remove PATH
+ },
+ };
+ }
+});
+```
+
+Return a `BashExecOverrides` object to override fields, or return `{ block: true, reason?: string }` to reject the command. Any field set to a non-undefined value replaces the original (`command`, `cwd`, `env`, `shell`, `args`, `timeout`). For `env`, set a key to `undefined` to remove it.
+
#### tool_result
-Fired after tool executes. **Can modify result.**
+Fired after tool executes. **Can modify result.** Use this to post-process outputs (for example, append hints or redact secrets) before the result is sent to the model.
```typescript
import { isBashToolResult } from "@mariozechner/pi-coding-agent";
@@ -565,10 +590,12 @@ pi.on("tool_result", async (event, ctx) => {
}
// Modify result:
- return { content: [...], details: {...}, isError: false };
+ return { content: [...], details: {...} };
});
```
+If `event.isError` is true, return `{ content: [...], isError: true }` to override the thrown error message (the text content becomes the error string). Returning `isError: true` on a successful tool result forces the tool to be treated as an error.
+
**Examples:** [git-checkpoint.ts](../examples/extensions/git-checkpoint.ts), [plan-mode/index.ts](../examples/extensions/plan-mode/index.ts)
### User Bash Events
diff --git a/packages/coding-agent/examples/extensions/uv.ts b/packages/coding-agent/examples/extensions/uv.ts
new file mode 100644
index 000000000..a1a240713
--- /dev/null
+++ b/packages/coding-agent/examples/extensions/uv.ts
@@ -0,0 +1,74 @@
+/**
+ * uv Python Interceptor
+ *
+ * Demonstrates before_bash_exec by redirecting python invocations through uv.
+ * This is a simple example that assumes basic whitespace-separated arguments.
+ *
+ * Usage:
+ * pi -e examples/extensions/uv.ts
+ */
+
+import { type ExtensionAPI, isBashToolResult } from "@mariozechner/pi-coding-agent";
+
+const PYTHON_PREFIX = /^python3?(\s+|$)/;
+const UV_RUN_PYTHON_PREFIX = /^uv\s+run\s+python3?(\s+|$)/;
+const PIP_PREFIX = /^pip3?(\s+|$)/;
+const PIP_MODULE_PATTERN = /\s-m\s+pip3?(\s|$)/;
+const TRACEBACK_PATTERN = /Traceback \(most recent call last\):/;
+const IMPORT_ERROR_PATTERN = /\b(ModuleNotFoundError|ImportError):/;
+const MODULE_NOT_FOUND_PATTERN = /No module named ['"]([^'"]+)['"]/;
+
+const PIP_BLOCK_REASON =
+ "pip is disabled. Use uv run instead, particularly --with and --script for throwaway work. Do not use uv pip!";
+
+export default function (pi: ExtensionAPI) {
+ pi.on("before_bash_exec", (event) => {
+ const trimmed = event.originalCommand.trim();
+ const isPythonCommand = PYTHON_PREFIX.test(trimmed);
+ const isUvRunPythonCommand = UV_RUN_PYTHON_PREFIX.test(trimmed);
+ const isPipModule = PIP_MODULE_PATTERN.test(trimmed);
+
+ if (PIP_PREFIX.test(trimmed) || (isPipModule && (isPythonCommand || isUvRunPythonCommand))) {
+ return {
+ block: true,
+ reason: PIP_BLOCK_REASON,
+ };
+ }
+
+ if (!isPythonCommand) {
+ return;
+ }
+
+ const normalizedCommand = trimmed.replace(PYTHON_PREFIX, "python ").trimEnd();
+ const uvCommand = `uv run ${normalizedCommand}`;
+
+ return {
+ command: uvCommand,
+ };
+ });
+
+ pi.on("tool_result", (event) => {
+ if (!isBashToolResult(event)) return;
+
+ const text = event.content
+ .filter((item) => item.type === "text")
+ .map((item) => item.text)
+ .join("");
+
+ if (!TRACEBACK_PATTERN.test(text) || !IMPORT_ERROR_PATTERN.test(text)) {
+ return;
+ }
+
+ const moduleMatch = text.match(MODULE_NOT_FOUND_PATTERN);
+ const moduleName = moduleMatch?.[1];
+ const hintTarget = moduleName ? ` --with ${moduleName}` : "";
+ const hint =
+ "\n\nHint: Python import failed. Use uv to fetch dependencies automatically without changing the system, " +
+ `e.g. \`uv run${hintTarget} python -c '...'\` or \`uv run --script\` for throwaway scripts.`;
+
+ return {
+ content: [...event.content, { type: "text", text: hint }],
+ isError: true,
+ };
+ });
+}
diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts
index f5c86f033..de4cba355 100644
--- a/packages/coding-agent/src/core/agent-session.ts
+++ b/packages/coding-agent/src/core/agent-session.ts
@@ -27,6 +27,7 @@ import { isContextOverflow, modelsAreEqual, supportsXhigh } from "@mariozechner/
import { getAuthPath } from "../config.js";
import { theme } from "../modes/interactive/theme/theme.js";
import { stripFrontmatter } from "../utils/frontmatter.js";
+import { getShellConfig, getShellEnv } from "../utils/shell.js";
import { type BashResult, executeBash as executeBashCommand, executeBashWithOperations } from "./bash-executor.js";
import {
type CompactionResult,
@@ -41,6 +42,7 @@ import {
import { exportSessionToHtml, type ToolHtmlRenderer } from "./export-html/index.js";
import { createToolHtmlRenderer } from "./export-html/tool-renderer.js";
import {
+ type BeforeBashExecEvent,
type ContextUsage,
type ExtensionCommandContextActions,
type ExtensionErrorListener,
@@ -2018,22 +2020,54 @@ export class AgentSession {
): Promise<BashResult> {
this._bashAbortController = new AbortController();
- // Apply command prefix if configured (e.g., "shopt -s expand_aliases" for alias support)
- const prefix = this.settingsManager.getShellCommandPrefix();
- const resolvedCommand = prefix ? `${prefix}\n${command}` : command;
-
try {
+ // Apply command prefix if configured (e.g., "shopt -s expand_aliases" for alias support)
+ const prefix = this.settingsManager.getShellCommandPrefix();
+ const resolvedCommand = prefix ? `${prefix}\n${command}` : command;
+ const shellConfig = getShellConfig();
+ const baseEvent: BeforeBashExecEvent = {
+ type: "before_bash_exec",
+ source: "user_bash",
+ command: resolvedCommand,
+ originalCommand: command,
+ cwd: process.cwd(),
+ env: { ...getShellEnv() },
+ shell: shellConfig.shell,
+ args: [...shellConfig.args],
+ };
+ const execEvent = this._extensionRunner?.hasHandlers("before_bash_exec")
+ ? await this._extensionRunner.emitBeforeBashExec(baseEvent)
+ : baseEvent;
+ const execCommand = execEvent.command;
+ const execCwd = execEvent.cwd;
+ const execEnv = execEvent.env;
+ const execShell = execEvent.shell;
+ const execArgs = execEvent.args;
+ const execTimeout = execEvent.timeout;
+
const result = options?.operations
- ? await executeBashWithOperations(resolvedCommand, process.cwd(), options.operations, {
+ ? await executeBashWithOperations(execCommand, execCwd, options.operations, {
onChunk,
signal: this._bashAbortController.signal,
+ env: execEnv,
+ shell: execShell,
+ args: execArgs,
+ timeout: execTimeout,
})
- : await executeBashCommand(resolvedCommand, {
+ : await executeBashCommand(execCommand, {
onChunk,
signal: this._bashAbortController.signal,
+ cwd: execCwd,
+ env: execEnv,
+ shell: execShell,
+ args: execArgs,
+ timeout: execTimeout,
});
- this.recordBashResult(command, result, options);
+ this.recordBashResult(command, result, {
+ excludeFromContext: options?.excludeFromContext,
+ executedCommand: execCommand === command ? undefined : execCommand,
+ });
return result;
} finally {
this._bashAbortController = undefined;
@@ -2044,10 +2078,15 @@ export class AgentSession {
* Record a bash execution result in session history.
* Used by executeBash and by extensions that handle bash execution themselves.
*/
- recordBashResult(command: string, result: BashResult, options?: { excludeFromContext?: boolean }): void {
+ recordBashResult(
+ command: string,
+ result: BashResult,
+ options?: { excludeFromContext?: boolean; executedCommand?: string },
+ ): void {
const bashMessage: BashExecutionMessage = {
role: "bashExecution",
command,
+ executedCommand: options?.executedCommand,
output: result.output,
exitCode: result.exitCode,
cancelled: result.cancelled,
diff --git a/packages/coding-agent/src/core/bash-executor.ts b/packages/coding-agent/src/core/bash-executor.ts
index b24982186..0d753426f 100644
--- a/packages/coding-agent/src/core/bash-executor.ts
+++ b/packages/coding-agent/src/core/bash-executor.ts
@@ -12,7 +12,7 @@ import { tmpdir } from "node:os";
import { join } from "node:path";
import { type ChildProcess, spawn } from "child_process";
import stripAnsi from "strip-ansi";
-import { getShellConfig, getShellEnv, killProcessTree, sanitizeBinaryOutput } from "../utils/shell.js";
+import { killProcessTree, resolveShellExecutionOptions, sanitizeBinaryOutput } from "../utils/shell.js";
import type { BashOperations } from "./tools/bash.js";
import { DEFAULT_MAX_BYTES, truncateTail } from "./tools/truncate.js";
@@ -25,6 +25,16 @@ export interface BashExecutorOptions {
onChunk?: (chunk: string) => void;
/** AbortSignal for cancellation */
signal?: AbortSignal;
+ /** Working directory override */
+ cwd?: string;
+ /** Environment override */
+ env?: NodeJS.ProcessEnv;
+ /** Shell executable override */
+ shell?: string;
+ /** Shell argument override */
+ args?: string[];
+ /** Timeout in seconds */
+ timeout?: number;
}
export interface BashResult {
@@ -60,13 +70,30 @@ export interface BashResult {
*/
export function executeBash(command: string, options?: BashExecutorOptions): Promise<BashResult> {
return new Promise((resolve, reject) => {
- const { shell, args } = getShellConfig();
- const child: ChildProcess = spawn(shell, [...args, command], {
+ const resolvedCwd = options?.cwd ?? process.cwd();
+ const { resolvedShell, resolvedArgs, resolvedEnv } = resolveShellExecutionOptions({
+ shell: options?.shell,
+ args: options?.args,
+ env: options?.env,
+ });
+ const child: ChildProcess = spawn(resolvedShell, [...resolvedArgs, command], {
+ cwd: resolvedCwd,
+ env: resolvedEnv,
detached: true,
- env: getShellEnv(),
stdio: ["ignore", "pipe", "pipe"],
});
+ let timedOut = false;
+ let timeoutHandle: NodeJS.Timeout | undefined;
+ if (options?.timeout !== undefined && options.timeout > 0) {
+ timeoutHandle = setTimeout(() => {
+ timedOut = true;
+ if (child.pid) {
+ killProcessTree(child.pid);
+ }
+ }, options.timeout * 1000);
+ }
+
// Track sanitized output for truncation
const outputChunks: string[] = [];
let outputBytes = 0;
@@ -88,6 +115,9 @@ export function executeBash(command: string, options?: BashExecutorOptions): Pro
if (options.signal.aborted) {
// Already aborted, don't even start
child.kill();
+ if (timeoutHandle) {
+ clearTimeout(timeoutHandle);
+ }
resolve({
output: "",
exitCode: undefined,
@@ -144,6 +174,9 @@ export function executeBash(command: string, options?: BashExecutorOptions): Pro
if (options?.signal) {
options.signal.removeEventListener("abort", abortHandler);
}
+ if (timeoutHandle) {
+ clearTimeout(timeoutHandle);
+ }
if (tempFileStream) {
tempFileStream.end();
@@ -153,8 +186,7 @@ export function executeBash(command: string, options?: BashExecutorOptions): Pro
const fullOutput = outputChunks.join("");
const truncationResult = truncateTail(fullOutput);
- // code === null means killed (cancelled)
- const cancelled = code === null;
+ const cancelled = code === null || timedOut;
resolve({
output: truncationResult.truncated ? truncationResult.content : fullOutput,
@@ -170,6 +202,9 @@ export function executeBash(command: string, options?: BashExecutorOptions): Pro
if (options?.signal) {
options.signal.removeEventListener("abort", abortHandler);
}
+ if (timeoutHandle) {
+ clearTimeout(timeoutHandle);
+ }
if (tempFileStream) {
tempFileStream.end();
@@ -238,6 +273,10 @@ export async function executeBashWithOperations(
const result = await operations.exec(command, cwd, {
onData,
signal: options?.signal,
+ timeout: options?.timeout,
+ env: options?.env,
+ shell: options?.shell,
+ args: options?.args,
});
if (tempFileStream) {
diff --git a/packages/coding-agent/src/core/extensions/index.ts b/packages/coding-agent/src/core/extensions/index.ts
index d10d0a1a5..d3ccbe248 100644
--- a/packages/coding-agent/src/core/extensions/index.ts
+++ b/packages/coding-agent/src/core/extensions/index.ts
@@ -25,9 +25,14 @@ export type {
// App keybindings (for custom editors)
AppAction,
AppendEntryHandler,
+ BashExecEvent,
+ BashExecOverrides,
+ BashExecSource,
BashToolResultEvent,
BeforeAgentStartEvent,
BeforeAgentStartEventResult,
+ BeforeBashExecEvent,
+ BeforeBashExecEventResult,
// Context
CompactOptions,
// Events - Agent
diff --git a/packages/coding-agent/src/core/extensions/runner.ts b/packages/coding-agent/src/core/extensions/runner.ts
index 289400a0b..31eb0410d 100644
--- a/packages/coding-agent/src/core/extensions/runner.ts
+++ b/packages/coding-agent/src/core/extensions/runner.ts
@@ -10,8 +10,12 @@ import type { KeyAction, KeybindingsConfig } from "../keybindings.js";
import type { ModelRegistry } from "../model-registry.js";
import type { SessionManager } from "../session-manager.js";
import type {
+ BashExecBlockResult,
+ BashExecOverrides,
BeforeAgentStartEvent,
BeforeAgentStartEventResult,
+ BeforeBashExecEvent,
+ BeforeBashExecEventResult,
CompactOptions,
ContextEvent,
ContextEventResult,
@@ -83,6 +87,34 @@ const buildBuiltinKeybindings = (effectiveKeybindings: Required<KeybindingsConfi
return builtinKeybindings;
};
+const applyBashExecOverrides = (event: BeforeBashExecEvent, overrides: BashExecOverrides): BeforeBashExecEvent => {
+ let nextEnv = event.env;
+ if (overrides.env) {
+ nextEnv = { ...event.env };
+ for (const [key, value] of Object.entries(overrides.env)) {
+ if (value === undefined) {
+ delete nextEnv[key];
+ } else {
+ nextEnv[key] = value;
+ }
+ }
+ }
+
+ return {
+ ...event,
+ command: overrides.command ?? event.command,
+ cwd: overrides.cwd ?? event.cwd,
+ env: nextEnv,
+ shell: overrides.shell ?? event.shell,
+ args: overrides.args ?? event.args,
+ timeout: overrides.timeout ?? event.timeout,
+ };
+};
+
+const isBashExecBlockResult = (result: BeforeBashExecEventResult): result is BashExecBlockResult => {
+ return "block" in result && result.block;
+};
+
/** Combined result from all before_agent_start handlers */
interface BeforeAgentStartCombinedResult {
messages?: NonNullable<BeforeAgentStartEventResult["message"]>[];
@@ -474,6 +506,50 @@ export class ExtensionRunner {
return result;
}
+ async emitBeforeBashExec(event: BeforeBashExecEvent): Promise<BeforeBashExecEvent> {
+ const ctx = this.createContext();
+ let currentEvent: BeforeBashExecEvent = {
+ ...event,
+ env: { ...event.env },
+ args: [...event.args],
+ };
+
+ for (const ext of this.extensions) {
+ const handlers = ext.handlers.get("before_bash_exec");
+ if (!handlers || handlers.length === 0) continue;
+
+ for (const handler of handlers) {
+ let handlerResult: unknown;
+ try {
+ handlerResult = await handler(currentEvent, ctx);
+ } catch (err) {
+ const message = err instanceof Error ? err.message : String(err);
+ const stack = err instanceof Error ? err.stack : undefined;
+ this.emitError({
+ extensionPath: ext.path,
+ event: "before_bash_exec",
+ error: message,
+ stack,
+ });
+ continue;
+ }
+
+ if (!handlerResult) {
+ continue;
+ }
+
+ const overrideResult = handlerResult as BeforeBashExecEventResult;
+ if (isBashExecBlockResult(overrideResult)) {
+ const reason = overrideResult.reason ?? "Bash execution was blocked by an extension";
+ throw new Error(reason);
+ }
+ currentEvent = applyBashExecOverrides(currentEvent, overrideResult);
+ }
+ }
+
+ return currentEvent;
+ }
+
async emitUserBash(event: UserBashEvent): Promise<UserBashEventResult | undefined> {
const ctx = this.createContext();
diff --git a/packages/coding-agent/src/core/extensions/types.ts b/packages/coding-agent/src/core/extensions/types.ts
index 2a058160e..e0c5bbe89 100644
--- a/packages/coding-agent/src/core/extensions/types.ts
+++ b/packages/coding-agent/src/core/extensions/types.ts
@@ -488,6 +488,43 @@ export interface UserBashEvent {
cwd: string;
}
+// ============================================================================
+// Bash Execution Events
+// ============================================================================
+
+export type BashExecSource = "tool" | "user_bash";
+
+export interface BashExecOverrides {
+ command?: string;
+ cwd?: string;
+ env?: Record<string, string | undefined>;
+ shell?: string;
+ args?: string[];
+ timeout?: number;
+}
+
+export interface BashExecBlockResult {
+ block: true;
+ reason?: string;
+}
+
+export interface BashExecEvent {
+ source: BashExecSource;
+ command: string;
+ originalCommand: string;
+ cwd: string;
+ env: NodeJS.ProcessEnv;
+ shell: string;
+ args: string[];
+ toolCallId?: string;
+ timeout?: number;
+}
+
+/** Fired before spawning a bash command (tool + user bash). */
+export interface BeforeBashExecEvent extends BashExecEvent {
+ type: "before_bash_exec";
+}
+
// ============================================================================
// Input Events
// ============================================================================
@@ -617,6 +654,7 @@ export type ExtensionEvent =
| TurnEndEvent
| ModelSelectEvent
| UserBashEvent
+ | BeforeBashExecEvent
| InputEvent
| ToolCallEvent
| ToolResultEvent;
@@ -642,6 +680,9 @@ export interface UserBashEventResult {
result?: BashResult;
}
+/** Result from before_bash_exec event handler */
+export type BeforeBashExecEventResult = BashExecOverrides | BashExecBlockResult;
+
export interface ToolResultEventResult {
content?: (TextContent | ImageContent)[];
details?: unknown;
@@ -749,6 +790,7 @@ export interface ExtensionAPI {
on(event: "tool_call", handler: ExtensionHandler<ToolCallEvent, ToolCallEventResult>): void;
on(event: "tool_result", handler: ExtensionHandler<ToolResultEvent, ToolResultEventResult>): void;
on(event: "user_bash", handler: ExtensionHandler<UserBashEvent, UserBashEventResult>): void;
+ on(event: "before_bash_exec", handler: ExtensionHandler<BeforeBashExecEvent, BeforeBashExecEventResult>): void;
on(event: "input", handler: ExtensionHandler<InputEvent, InputEventResult>): void;
// =========================================================================
diff --git a/packages/coding-agent/src/core/extensions/wrapper.ts b/packages/coding-agent/src/core/extensions/wrapper.ts
index 0626afaf5..f38293e75 100644
--- a/packages/coding-agent/src/core/extensions/wrapper.ts
+++ b/packages/coding-agent/src/core/extensions/wrapper.ts
@@ -3,8 +3,10 @@
*/
import type { AgentTool, AgentToolUpdateCallback } from "@mariozechner/pi-agent-core";
+import type { ImageContent, TextContent } from "@mariozechner/pi-ai";
+import { getShellConfig, getShellEnv } from "../../utils/shell.js";
import type { ExtensionRunner } from "./runner.js";
-import type { RegisteredTool, ToolCallEventResult, ToolResultEventResult } from "./types.js";
+import type { BeforeBashExecEvent, RegisteredTool, ToolCallEventResult, ToolResultEventResult } from "./types.js";
/**
* Wrap a RegisteredTool into an AgentTool.
@@ -36,6 +38,60 @@ export function wrapRegisteredTools(registeredTools: RegisteredTool[], runner: E
* - Emits tool_result event after execution (can modify result)
*/
export function wrapToolWithExtensions<T>(tool: AgentTool<any, T>, runner: ExtensionRunner): AgentTool<any, T> {
+ type BashToolParams = {
+ command: string;
+ timeout?: number;
+ };
+ type BashExecParams = BashToolParams & {
+ cwd?: string;
+ env?: NodeJS.ProcessEnv;
+ shell?: string;
+ args?: string[];
+ };
+ const applyBeforeBashExecOverrides = async (
+ toolCallId: string,
+ params: BashToolParams,
+ runner: ExtensionRunner,
+ ): Promise<BashExecParams> => {
+ const shellConfig = getShellConfig();
+ const context = runner.createContext();
+ const baseEvent: BeforeBashExecEvent = {
+ type: "before_bash_exec",
+ source: "tool",
+ command: params.command,
+ originalCommand: params.command,
+ cwd: context.cwd,
+ env: { ...getShellEnv() },
+ shell: shellConfig.shell,
+ args: [...shellConfig.args],
+ toolCallId,
+ timeout: params.timeout,
+ };
+ const execEvent = await runner.emitBeforeBashExec(baseEvent);
+ return {
+ ...params,
+ command: execEvent.command,
+ cwd: execEvent.cwd,
+ env: execEvent.env,
+ shell: execEvent.shell,
+ args: execEvent.args,
+ timeout: execEvent.timeout,
+ };
+ };
+ const toolResultContentToErrorMessage = (
+ content: (TextContent | ImageContent)[] | undefined,
+ fallback: string,
+ ): string => {
+ if (!content || content.length === 0) return fallback;
+ const text = content
+ .filter((item): item is TextContent => item.type === "text" && !!item.text)
+ .map((item) => item.text)
+ .join("")
+ .trim();
+ if (text) return text;
+ return `${fallback} [non-text content]`;
+ };
+
return {
...tool,
execute: async (
@@ -44,6 +100,9 @@ export function wrapToolWithExtensions<T>(tool: AgentTool<any, T>, runner: Exten
signal?: AbortSignal,
onUpdate?: AgentToolUpdateCallback<T>,
) => {
+ let effectiveParams = params;
+ let forcedError = false;
+
// Emit tool_call event - extensions can block execution
if (runner.hasHandlers("tool_call")) {
try {
@@ -66,9 +125,13 @@ export function wrapToolWithExtensions<T>(tool: AgentTool<any, T>, runner: Exten
}
}
+ if (tool.name === "bash" && runner.hasHandlers("before_bash_exec")) {
+ effectiveParams = await applyBeforeBashExecOverrides(toolCallId, params as BashToolParams, runner);
+ }
+
// Execute the actual tool
try {
- const result = await tool.execute(toolCallId, params, signal, onUpdate);
+ const result = await tool.execute(toolCallId, effectiveParams, signal, onUpdate);
// Emit tool_result event - extensions can modify the result
if (runner.hasHandlers("tool_result")) {
@@ -76,33 +139,53 @@ export function wrapToolWithExtensions<T>(tool: AgentTool<any, T>, runner: Exten
type: "tool_result",
toolName: tool.name,
toolCallId,
- input: params,
+ input: effectiveParams,
content: result.content,
details: result.details,
isError: false,
})) as ToolResultEventResult | undefined;
if (resultResult) {
+ const nextContent = resultResult.content ?? result.content;
+ const nextDetails = (resultResult.details ?? result.details) as T;
+ if (resultResult.isError) {
+ forcedError = true;
+ throw new Error(toolResultContentToErrorMessage(nextContent, "Tool execution failed."));
+ }
+
return {
- content: resultResult.content ?? result.content,
- details: (resultResult.details ?? result.details) as T,
+ content: nextContent,
+ details: nextDetails,
};
}
}
return result;
} catch (err) {
+ if (forcedError) {
+ throw err;
+ }
// Emit tool_result event for errors
if (runner.hasHandlers("tool_result")) {
- await runner.emit({
+ const fallbackMessage = err instanceof Error ? err.message : String(err);
+ const content = [{ type: "text" as const, text: fallbackMessage }];
+ const resultResult = (await runner.emit({
type: "tool_result",
toolName: tool.name,
toolCallId,
- input: params,
- content: [{ type: "text", text: err instanceof Error ? err.message : String(err) }],
+ input: effectiveParams,
+ content,
details: undefined,
isError: true,
- });
+ })) as ToolResultEventResult | undefined;
+
+ if (resultResult) {
+ if (!resultResult.isError) {
+ throw err;
+ }
+ const nextContent = resultResult.content ?? content;
+ throw new Error(toolResultContentToErrorMessage(nextContent, fallbackMessage));
+ }
}
throw err;
}
diff --git a/packages/coding-agent/src/core/messages.ts b/packages/coding-agent/src/core/messages.ts
index f5a645e66..24106f3b4 100644
--- a/packages/coding-agent/src/core/messages.ts
+++ b/packages/coding-agent/src/core/messages.ts
@@ -29,6 +29,7 @@ export const BRANCH_SUMMARY_SUFFIX = `</summary>`;
export interface BashExecutionMessage {
role: "bashExecution";
command: string;
+ executedCommand?: string;
output: string;
exitCode: number | undefined;
cancelled: boolean;
diff --git a/packages/coding-agent/src/core/tools/bash.ts b/packages/coding-agent/src/core/tools/bash.ts
index 3c312e52b..b9ea6a9e2 100644
--- a/packages/coding-agent/src/core/tools/bash.ts
+++ b/packages/coding-agent/src/core/tools/bash.ts
@@ -5,7 +5,7 @@ import { join } from "node:path";
import type { AgentTool } from "@mariozechner/pi-agent-core";
import { Type } from "@sinclair/typebox";
import { spawn } from "child_process";
-import { getShellConfig, getShellEnv, killProcessTree } from "../../utils/shell.js";
+import { killProcessTree, resolveShellExecutionOptions } from "../../utils/shell.js";
import { DEFAULT_MAX_BYTES, DEFAULT_MAX_LINES, formatSize, type TruncationResult, truncateTail } from "./truncate.js";
/**
@@ -45,6 +45,9 @@ export interface BashOperations {
onData: (data: Buffer) => void;
signal?: AbortSignal;
timeout?: number;
+ env?: NodeJS.ProcessEnv;
+ shell?: string;
+ args?: string[];
},
) => Promise<{ exitCode: number | null }>;
}
@@ -53,19 +56,23 @@ export interface BashOperations {
* Default bash operations using local shell
*/
const defaultBashOperations: BashOperations = {
- exec: (command, cwd, { onData, signal, timeout }) => {
+ exec: (command, cwd, { onData, signal, timeout, env, shell, args }) => {
return new Promise((resolve, reject) => {
- const { shell, args } = getShellConfig();
+ const { resolvedShell, resolvedArgs, resolvedEnv } = resolveShellExecutionOptions({
+ shell,
+ args,
+ env,
+ });
if (!existsSync(cwd)) {
reject(new Error(`Working directory does not exist: ${cwd}\nCannot execute bash commands.`));
return;
}
- const child = spawn(shell, [...args, command], {
+ const child = spawn(resolvedShell, [...resolvedArgs, command], {
cwd,
+ env: resolvedEnv,
detached: true,
- env: getShellEnv(),
stdio: ["ignore", "pipe", "pipe"],
});
@@ -151,12 +158,27 @@ export function createBashTool(cwd: string, options?: BashToolOptions): AgentToo
parameters: bashSchema,
execute: async (
_toolCallId: string,
- { command, timeout }: { command: string; timeout?: number },
+ {
+ command,
+ timeout,
+ cwd: overrideCwd,
+ env,
+ shell,
+ args,
+ }: {
+ command: string;
+ timeout?: number;
+ cwd?: string;
+ env?: NodeJS.ProcessEnv;
+ shell?: string;
+ args?: string[];
+ },
signal?: AbortSignal,
onUpdate?,
) => {
// Apply command prefix if configured (e.g., "shopt -s expand_aliases" for alias support)
const resolvedCommand = commandPrefix ? `${commandPrefix}\n${command}` : command;
+ const resolvedCwd = overrideCwd ?? cwd;
return new Promise((resolve, reject) => {
// We'll stream to a temp file if output gets large
@@ -213,7 +235,7 @@ export function createBashTool(cwd: string, options?: BashToolOptions): AgentToo
}
};
- ops.exec(resolvedCommand, cwd, { onData: handleData, signal, timeout })
+ ops.exec(resolvedCommand, resolvedCwd, { onData: handleData, signal, timeout, env, shell, args })
.then(({ exitCode }) => {
// Close temp file stream
if (tempFileStream) {
diff --git a/packages/coding-agent/src/index.ts b/packages/coding-agent/src/index.ts
index ecf08a77e..dfdb79d38 100644
--- a/packages/coding-agent/src/index.ts
+++ b/packages/coding-agent/src/index.ts
@@ -44,7 +44,12 @@ export type {
AgentToolResult,
AgentToolUpdateCallback,
AppAction,
+ BashExecEvent,
+ BashExecOverrides,
+ BashExecSource,
BeforeAgentStartEvent,
+ BeforeBashExecEvent,
+ BeforeBashExecEventResult,
CompactOptions,
ContextEvent,
ContextUsage,
diff --git a/packages/coding-agent/src/utils/shell.ts b/packages/coding-agent/src/utils/shell.ts
index 62ff558af..3c157c8d3 100644
--- a/packages/coding-agent/src/utils/shell.ts
+++ b/packages/coding-agent/src/utils/shell.ts
@@ -1,8 +1,7 @@
import { existsSync } from "node:fs";
import { delimiter } from "node:path";
import { spawn, spawnSync } from "child_process";
-import { getSettingsPath } from "../config.js";
-import { getBinDir } from "../config.js";
+import { getBinDir, getSettingsPath } from "../config.js";
import { SettingsManager } from "../core/settings-manager.js";
let cachedShellConfig: { shell: string; args: string[] } | null = null;
@@ -111,6 +110,34 @@ export function getShellEnv(): NodeJS.ProcessEnv {
};
}
+export interface ShellExecutionOptions {
+ shell?: string;
+ args?: string[];
+ env?: NodeJS.ProcessEnv;
+}
+
+export interface ResolvedShellExecutionOptions {
+ resolvedShell: string;
+ resolvedArgs: string[];
+ resolvedEnv: NodeJS.ProcessEnv;
+}
+
+export function resolveShellExecutionOptions(options?: ShellExecutionOptions): ResolvedShellExecutionOptions {
+ const shellConfig = getShellConfig();
+ const resolvedEnv = options?.env ? { ...options.env } : { ...getShellEnv() };
+ for (const [key, value] of Object.entries(resolvedEnv)) {
+ if (value === undefined) {
+ delete resolvedEnv[key];
+ }
+ }
+
+ return {
+ resolvedShell: options?.shell ?? shellConfig.shell,
+ resolvedArgs: options?.args ?? shellConfig.args,
+ resolvedEnv,
+ };
+}
+
/**
* Sanitize binary output for display/storage.
* Removes characters that crash string-width or cause display issues:
diff --git a/packages/coding-agent/test/tools.test.ts b/packages/coding-agent/test/tools.test.ts
index 851633d22..83bfc44ce 100644
--- a/packages/coding-agent/test/tools.test.ts
+++ b/packages/coding-agent/test/tools.test.ts
@@ -290,9 +290,10 @@ describe("Coding Agent Tools", () => {
});
it("should handle process spawn errors", async () => {
- vi.spyOn(shellModule, "getShellConfig").mockReturnValueOnce({
- shell: "/nonexistent-shell-path-xyz123",
- args: ["-c"],
+ vi.spyOn(shellModule, "resolveShellExecutionOptions").mockReturnValueOnce({
+ resolvedShell: "/nonexistent-shell-path-xyz123",
+ resolvedArgs: ["-c"],
+ resolvedEnv: shellModule.getShellEnv(),
});
const bashWithBadShell = createBashTool(testDir);Let me now check for linked issues and read the current state of relevant files:
>cd /Users/badlogic/workspaces/pi-mono && gh issue view 903 --json title,body,comments,labels,state 2>/dev/null || echo "No issue #903 found"{"body":"Discussion:\r\n\r\n<img width=\"1020\" height=\"350\" alt=\"image\" src=\"https://github.com/user-attachments/assets/a2a1633a-acee-40d8-a7ba-2d2ab9808514\" />\r\n\r\nThis allows hooking bash execution and it also allows `tool_result` hooks to manipulate error messages by \"abusing\" content to carry error messages. This feels somewhat consistent to how it works but a bit ugly.","comments":[{"id":"IC_kwDOPbFNk87huidp","author":{"login":"cv"},"authorAssociation":"CONTRIBUTOR","body":"Happy to modify/rewrite the examples/ssh.ts (and my own pi-ssh-remote) when this lands.","createdAt":"2026-01-22T22:32:17Z","includesCreatedEdit":false,"isMinimized":false,"minimizedReason":"","reactionGroups":[],"url":"https://github.com/badlogic/pi-mono/pull/903#issuecomment-3787073385","viewerDidAuthor":false},{"id":"IC_kwDOPbFNk87h1et-","author":{"login":"mitsuhiko"},"authorAssociation":"CONTRIBUTOR","body":"@cv it should in theory work with it already, the question in a way is if ssh.ts can further benefit from the changes.","createdAt":"2026-01-23T08:01:00Z","includesCreatedEdit":false,"isMinimized":false,"minimizedReason":"","reactionGroups":[],"url":"https://github.com/badlogic/pi-mono/pull/903#issuecomment-3788893054","viewerDidAuthor":false},{"id":"IC_kwDOPbFNk87h5XIx","author":{"login":"dejanr"},"authorAssociation":"NONE","body":"This could be really nice for project purposes, instead of blowing agents.md and instructions on how to run sandboxed environment, we could just build custom extensions that are preparing env for tool, is that nix develop, or uv, or whatever else that someone uses.\r\n\r\nFrom architectural point of view i would prefer a middleware approach to tool wrapping, then dangling handlers.","createdAt":"2026-01-23T11:56:52Z","includesCreatedEdit":true,"isMinimized":false,"minimizedReason":"","reactionGroups":[],"url":"https://github.com/badlogic/pi-mono/pull/903#issuecomment-3789910577","viewerDidAuthor":false},{"id":"IC_kwDOPbFNk87iPMHC","author":{"login":"mitsuhiko"},"authorAssociation":"CONTRIBUTOR","body":"@dejanr @Mic92 I think it would be nice if people other than me would test drive this and give some suggestions. It's quite narrowly scoped to what i tried so far.","createdAt":"2026-01-24T22:16:42Z","includesCreatedEdit":false,"isMinimized":false,"minimizedReason":"","reactionGroups":[],"url":"https://github.com/badlogic/pi-mono/pull/903#issuecomment-3795632578","viewerDidAuthor":false}],"labels":[],"state":"OPEN","title":"Add support for intercepting bash"}Now let me read the relevant source files to understand the existing implementation:
Let me also read the bash-executor and agent-session files:
Now let me read the tools/bash.ts file to see the current implementation:
Now let me check the test file referenced in the PR:
Now let me check the index exports:
Now I have a good understanding of the PR. Let me review the README and docs to see if documentation changes are needed:
>cd /Users/badlogic/workspaces/pi-mono && rg -l "before_bash_exec\|user_bash" packages/coding-agent/examples/extensions/ 2>/dev/null || echo "No matches"No matches>cd /Users/badlogic/workspaces/pi-mono && cat packages/coding-agent/examples/extensions/ssh.ts 2>/dev/null | head -100/**
* SSH Remote Execution Example
*
* Demonstrates delegating tool operations to a remote machine via SSH.
* When --ssh is provided, read/write/edit/bash run on the remote.
*
* Usage:
* pi -e ./ssh.ts --ssh user@host
* pi -e ./ssh.ts --ssh user@host:/remote/path
*
* Requirements:
* - SSH key-based auth (no password prompts)
* - bash on remote
*/
import { spawn } from "node:child_process";
import type { ExtensionAPI } from "@mariozechner/pi-coding-agent";
import {
type BashOperations,
createBashTool,
createEditTool,
createReadTool,
createWriteTool,
type EditOperations,
type ReadOperations,
type WriteOperations,
} from "@mariozechner/pi-coding-agent";
function sshExec(remote: string, command: string): Promise<Buffer> {
return new Promise((resolve, reject) => {
const child = spawn("ssh", [remote, command], { stdio: ["ignore", "pipe", "pipe"] });
const chunks: Buffer[] = [];
const errChunks: Buffer[] = [];
child.stdout.on("data", (data) => chunks.push(data));
child.stderr.on("data", (data) => errChunks.push(data));
child.on("error", reject);
child.on("close", (code) => {
if (code !== 0) {
reject(new Error(`SSH failed (${code}): ${Buffer.concat(errChunks).toString()}`));
} else {
resolve(Buffer.concat(chunks));
}
});
});
}
function createRemoteReadOps(remote: string, remoteCwd: string, localCwd: string): ReadOperations {
const toRemote = (p: string) => p.replace(localCwd, remoteCwd);
return {
readFile: (p) => sshExec(remote, `cat ${JSON.stringify(toRemote(p))}`),
access: (p) => sshExec(remote, `test -r ${JSON.stringify(toRemote(p))}`).then(() => {}),
detectImageMimeType: async (p) => {
try {
const r = await sshExec(remote, `file --mime-type -b ${JSON.stringify(toRemote(p))}`);
const m = r.toString().trim();
return ["image/jpeg", "image/png", "image/gif", "image/webp"].includes(m) ? m : null;
} catch {
return null;
}
},
};
}
function createRemoteWriteOps(remote: string, remoteCwd: string, localCwd: string): WriteOperations {
const toRemote = (p: string) => p.replace(localCwd, remoteCwd);
return {
writeFile: async (p, content) => {
const b64 = Buffer.from(content).toString("base64");
await sshExec(remote, `echo ${JSON.stringify(b64)} | base64 -d > ${JSON.stringify(toRemote(p))}`);
},
mkdir: (dir) => sshExec(remote, `mkdir -p ${JSON.stringify(toRemote(dir))}`).then(() => {}),
};
}
function createRemoteEditOps(remote: string, remoteCwd: string, localCwd: string): EditOperations {
const r = createRemoteReadOps(remote, remoteCwd, localCwd);
const w = createRemoteWriteOps(remote, remoteCwd, localCwd);
return { readFile: r.readFile, access: r.access, writeFile: w.writeFile };
}
function createRemoteBashOps(remote: string, remoteCwd: string, localCwd: string): BashOperations {
const toRemote = (p: string) => p.replace(localCwd, remoteCwd);
return {
exec: (command, cwd, { onData, signal, timeout }) =>
new Promise((resolve, reject) => {
const cmd = `cd ${JSON.stringify(toRemote(cwd))} && ${command}`;
const child = spawn("ssh", [remote, cmd], { stdio: ["ignore", "pipe", "pipe"] });
let timedOut = false;
const timer = timeout
? setTimeout(() => {
timedOut = true;
child.kill();
}, timeout * 1000)
: undefined;
child.stdout.on("data", onData);
child.stderr.on("data", onData);
child.on("error", (e) => {
if (timer) clearTimeout(timer);
reject(e);
});Now I have a comprehensive understanding of the PR. Let me provide the review:
PR: https://github.com/badlogic/pi-mono/pull/903
Changelog:
packages/coding-agent/CHANGELOG.md. A changelog entry is required before merge. If the user decides to merge, I will add it.Good:
before_bash_exec event gives extensions a unified hook for both tool-based and user (!/!!) bash executionuv.ts example demonstrates a practical use case (Python/uv interception) and serves as good documentationBashExecBlockResult allows extensions to block bash commands with custom reasonsenv override with undefined to remove keys is intuitiveoriginalCommand field preserves the original command for logging/debugging while allowing command modificationisError flag in tool_result allows extensions to convert successful tool results to errors (useful for policy enforcement)BashExecEvent, BashExecOverrides, BashExecSource, BeforeBashExecEvent, BeforeBashExecEventResultextensions.md and README.md is clear with good examplesresolveShellExecutionOptions instead of getShellConfig to match the new implementationBad:
The PR extends BashOperations.exec interface with new optional parameters (env, shell, args) but doesn't update all callers. The executeBashWithOperations function passes these new parameters to operations.exec, but existing implementations (like in ssh.ts example) don't expect them. This could break extensions using custom BashOperations.
The BashExecutionMessage.executedCommand field is added but never read/used anywhere in the codebase. It's stored in session history but not surfaced in UI or used for replay.
The tool_result error handling in wrapper.ts has complex branching logic for forcedError and isError. The flow is:
isError: true on success, throw error with content as messageisError: true, the original error is rethrown (content ignored)isError: true on existing error, content becomes new error messageThis is confusing and the !resultResult.isError check on line 167 seems inverted (it throws the original error when NOT an error, but that's inside the catch block for errors).
Ugly:
applyBeforeBashExecOverrides function in wrapper.ts is defined inside wrapToolWithExtensions, creating a new function definition on every call. This should be hoisted outside.resolveShellExecutionOptions in shell.ts has redundant undefined-deletion loop. If options?.env is passed, we create a new object from it, then iterate to delete undefined keys. But the delete-undefined loop also runs on getShellEnv() fallback which shouldn't have undefined values.Questions or Assumptions:
executedCommand field be displayed in the bash execution UI component when it differs from command? Currently it's stored but invisible.before_bash_exec handlers are chained? The implementation chains them (currentEvent = applyBashExecOverrides(currentEvent, overrideResult)), but this isn't documented.BashResult with an error message instead of throwing? The current behavior throws which may not provide the best UX in all cases.timeout override in before_bash_exec could allow extensions to extend or override timeouts. Is this intentional?Change summary:
before_bash_exec extension event fired before bash execution (both tool and user !/!!)executedCommand to session history when command is rewrittentool_result event to allow modifying error messages via isError: true + contentresolveShellExecutionOptions helper for unified shell config resolutionuv.ts example extension demonstrating Python/uv interceptionextensions.md and README.mdTests:
resolveShellExecutionOptions instead of getShellConfig for spawn error testbefore_bash_exec event handlertool_result error manipulation flowbefore_bash_exec handlers{ block: true }what is executeCommand supposed to do?
Looking at the PR diff, executedCommand (not executeCommand) is a new optional field added to BashExecutionMessage:
export interface BashExecutionMessage { role: "bashExecution"; command: string; executedCommand?: string; // NEW output: string; exitCode: number | undefined; // ...}It's set in recordBashResult() when the command was rewritten by before_bash_exec handlers:
this.recordBashResult(command, result, { excludeFromContext: options?.excludeFromContext, executedCommand: execCommand === command ? undefined : execCommand,});The purpose is to preserve what command was actually executed when it differs from the original. For example, with the uv.ts extension:
python script.pybefore_bash_exec rewrites to: uv run python script.pycommand: "python script.py", executedCommand: "uv run python script.py"This allows:
However, as I noted in the review, the field is stored but never actually used anywhere. The BashExecutionComponent doesn't render it, and there's no logic that reads it back. It's essentially dead data in the current implementation.
this seems like a shitty PR overall
Yeah, it's rough. The core problems:
Incomplete implementation - executedCommand is stored but never displayed or used. Dead code.
Breaking change to BashOperations - Adds env, shell, args params to the interface without updating existing implementations like ssh.ts. Extensions using custom BashOperations will silently ignore these overrides.
No tests - Zero tests for the new before_bash_exec event, blocking, chaining, or the tool_result error manipulation.
Confusing error handling - The tool_result isError logic in wrapper.ts is convoluted. The PR author admits the approach is "ugly" in the description.
Function defined inside function - applyBeforeBashExecOverrides recreated on every tool call.
Scope creep - Mixes two unrelated features: bash interception AND tool_result error manipulation. Should be separate PRs.
The uv.ts example is nice, but it's lipstick on a pig. The feature isn't production-ready without tests and the BashOperations interface change needs a migration path or at least documentation about breaking existing extensions.
I'd request significant revisions or close it and ask for a cleaner implementation.
can we have a cleaner, more concise implementation? explain the error handling in detail to me first
The error handling in wrapper.ts has two paths:
Success path (tool executed without throwing):
const result = await tool.execute(...);
if (runner.hasHandlers("tool_result")) { const resultResult = await runner.emit({..., isError: false}); if (resultResult) { if (resultResult.isError) { // Extension wants to FORCE this successful result into an error forcedError = true; throw new Error(contentToString(resultResult.content)); } // Otherwise just modify content/details return { content: resultResult.content, details: resultResult.details }; }}return result;Error path (tool threw):
catch (err) { if (forcedError) throw err; // Already handled above, just rethrow if (runner.hasHandlers("tool_result")) { const resultResult = await runner.emit({..., isError: true}); if (resultResult) { if (!resultResult.isError) { // Extension returned content but didn't set isError: true // So we ignore the content and rethrow original throw err; } // Extension wants to REPLACE the error message throw new Error(contentToString(resultResult.content)); } } throw err;}The confusion:
isError: true in the result converts success to errorisError: true to replace the error message, otherwise your content is ignoredThe !resultResult.isError check on line 167 says: "if the extension responded to an error event but didn't explicitly say isError: true, ignore whatever they returned and throw the original error."
This is defensive but confusing. Why emit the event if you're going to ignore the response?
what in the fuck, i don't understand what this is trying to do, what's the use case? how'd that look?
From the PR description and the uv.ts example, here's the use case:
Scenario: Python script fails with ModuleNotFoundError. The extension wants to append a hint to the error message telling the model to use uv run --with <module>.
// uv.ts example from the PRpi.on("tool_result", (event) => { if (!isBashToolResult(event)) return; // Check if output has Python import error if (!TRACEBACK_PATTERN.test(text) || !IMPORT_ERROR_PATTERN.test(text)) { return; } // Append hint to the error const hint = "\n\nHint: Use uv run --with <module> ..."; return { content: [...event.content, { type: "text", text: hint }], isError: true, // <-- Must set this or hint is ignored };});The problem: Bash tool throws on non-zero exit. The extension catches tool_result with isError: true, wants to modify the error message. But the current API requires you to return isError: true again or your modifications are silently discarded.
Simpler design:
Just let extensions modify content regardless. If the tool errored, the modified content becomes the new error message. If it succeeded, the modified content is the new result. No isError flag gymnastics.
pi.on("tool_result", (event) => { if (event.isError && isPythonImportError(event.content)) { return { content: [...event.content, { type: "text", text: hint }] }; }});The whole isError return flag is overengineered. The only legitimate use case for changing error status would be converting success to error (policy blocking), which could be a separate { block: true, reason: "..." } return like tool_call already has.
return { content: [...event.content, { type: "text", text: hint }], isError: true, // <-- Must set this or hint is ignored };
why in the fuck do we need isError: true here? tool_result event handler can modify the returned result without having to rewrite the error state? the model will receive the additional content just fine, no?
Exactly. The isError flag is completely unnecessary for this use case.
When bash exits non-zero, the tool throws. The error message becomes the tool result content sent to the model. If an extension appends a hint to that content, the model receives the modified content. Done.
The model doesn't care whether it was an "error" or not. It just sees the text content in the tool result. Whether that text says "Command failed" or "Command failed\n\nHint: try X" makes no difference to the protocol.
The only place isError matters is internally: do we throw or return? But the extension shouldn't control that. The tool already decided by throwing or not.
So the entire isError machinery in this PR is pointless complexity. Extensions should just be able to:
pi.on("tool_result", (event) => { if (shouldAddHint(event)) { return { content: [...event.content, { type: "text", text: hint }] }; }});And that content modification should work regardless of whether the tool succeeded or failed. The PR overengineered this by conflating "modify the content" with "change the error state."
what other inconsistencies are there?
Looking at the PR again:
1. before_bash_exec vs user_bash overlap
There's already a user_bash event that fires for !/!! commands. Now before_bash_exec also fires for user bash with source: "user_bash". Two events for the same thing, different capabilities:
user_bash can return custom BashOperations or a full BashResultbefore_bash_exec can modify command/env/cwd but not provide custom operationsWhy not just extend user_bash?
2. Tool bash gets before_bash_exec, but not custom operations
For user bash, you can provide custom BashOperations via user_bash event. For tool bash, you can only modify parameters via before_bash_exec. No way to redirect tool bash to SSH without replacing the entire tool.
3. BashOperations interface extended but not consistently
The PR adds env, shell, args to the exec call in defaultBashOperations, but BashOperations interface signature in the types still shows:
exec: (command, cwd, { onData, signal, timeout }) => ...Extensions implementing custom BashOperations won't receive the new params.
4. executedCommand stored but invisible
As discussed. Dead field.
5. Timeout handling inconsistent
before_bash_exec can override timeout, but for user bash (agent-session.ts), the timeout isn't even a concept. User bash has no timeout parameter to override.
6. No way to observe what actually executed
Extensions can modify the command, but there's no after_bash_exec event to see the final result with the modified command context.
what would be a better implementation? more concise, more consistent?
while still supporting what armin does in his uv.ts stuff. note that before_bash_exec is composable as oposed to user_bash or overwriting the bash tool, which i believe is the point of armin's exercise
The core insight is right: a composable hook that intercepts bash before execution, works for both tool and user bash, and lets multiple extensions chain modifications.
Simplified design:
before_bash_exec - fires for all bash (tool + user), composableisError return gymnastics - tool_result just modifies content, periodexecutedCommand storage - if you need it, add an after_bash_exec event laterBashOperations interface - that's a different feature (remote execution)The event:
interface BeforeBashExecEvent { type: "before_bash_exec"; source: "tool" | "user"; command: string; cwd: string; env: Record<string, string>; timeout?: number;}
type BeforeBashExecResult = | { block: true; reason?: string } | { command?: string; cwd?: string; env?: Record<string, string>; timeout?: number };No shell, no args (internal details). No originalCommand (handler already has it in the event). Handlers chain by mutating fields they care about.
Implementation:
One function emitBeforeBashExec() in runner. Call it from:
wrapToolWithExtensions for bash tool (before execute)agent-session.executeBash for user bash (before execute)For tool_result:
Just remove the isError return field entirely. Extensions return { content } to modify, that's it. The tool already threw or didn't. The model receives whatever content you return.
This cuts the PR roughly in half and removes all the confusing parts while keeping the composable interception that makes uv.ts work.
export interface BashExecutionMessage { role: "bashExecution"; command: string; executedCommand?: string; // NEW output: string; exitCode: number | undefined; // ...}this.recordBashResult(command, result, { excludeFromContext: options?.excludeFromContext, executedCommand: execCommand === command ? undefined : execCommand,});const result = await tool.execute(...);
if (runner.hasHandlers("tool_result")) { const resultResult = await runner.emit({..., isError: false}); if (resultResult) { if (resultResult.isError) { // Extension wants to FORCE this successful result into an error forcedError = true; throw new Error(contentToString(resultResult.content)); } // Otherwise just modify content/details return { content: resultResult.content, details: resultResult.details }; }}return result;catch (err) { if (forcedError) throw err; // Already handled above, just rethrow if (runner.hasHandlers("tool_result")) { const resultResult = await runner.emit({..., isError: true}); if (resultResult) { if (!resultResult.isError) { // Extension returned content but didn't set isError: true // So we ignore the content and rethrow original throw err; } // Extension wants to REPLACE the error message throw new Error(contentToString(resultResult.content)); } } throw err;}// uv.ts example from the PRpi.on("tool_result", (event) => { if (!isBashToolResult(event)) return; // Check if output has Python import error if (!TRACEBACK_PATTERN.test(text) || !IMPORT_ERROR_PATTERN.test(text)) { return; } // Append hint to the error const hint = "\n\nHint: Use uv run --with <module> ..."; return { content: [...event.content, { type: "text", text: hint }], isError: true, // <-- Must set this or hint is ignored };});pi.on("tool_result", (event) => { if (event.isError && isPythonImportError(event.content)) { return { content: [...event.content, { type: "text", text: hint }] }; }});pi.on("tool_result", (event) => { if (shouldAddHint(event)) { return { content: [...event.content, { type: "text", text: hint }] }; }});exec: (command, cwd, { onData, signal, timeout }) => ...interface BeforeBashExecEvent { type: "before_bash_exec"; source: "tool" | "user"; command: string; cwd: string; env: Record<string, string>; timeout?: number;}
type BeforeBashExecResult = | { block: true; reason?: string } | { command?: string; cwd?: string; env?: Record<string, string>; timeout?: number };