Reviewed and proposed improvements for bash execution PR

Summary

In Progress

Understand
Reviewed PR details, linked issues, and code changes.
Understand
Understand
Analyzed error handling and use cases.
Understand
Understand
Identified inconsistencies and design flaws.
Understand
Plan
Proposed 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:

Read the PR page in full. Include description, all comments, all commits, and all changed files.
Identify any linked issues referenced in the PR body, comments, commit messages, or cross links. Read each issue in full, including all comments.
Analyze the PR diff. Read all relevant code files in full with no truncation from the current main branch and compare against the diff. Do not fetch PR file blobs unless a file is missing on main or the diff context is insufficient. Include related code paths that are not in the diff but are required to validate behavior.
Check for a changelog entry in the relevant 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:
- Entry uses correct section (### Breaking Changes, ### Added, ### Fixed, etc.)
- External contributions include PR link and author: Fixed foo ([#123](https://github.com/badlogic/pi-mono/pull/123) by [@user](https://github.com/user))
- Breaking changes are in ### Breaking Changes, not just ### Fixed
Check if packages/coding-agent/README.md, packages/coding-agent/docs/.md, packages/coding-agent/examples/**/.md require modification. This is usually the case when existing features have been changed, or new features have been added.
Provide a structured review with these sections:
- Good: solid choices or improvements
- Bad: concrete issues, regressions, missing tests, or risks
- Ugly: subtle or high impact problems
Add Questions or Assumptions if anything is unclear.
Add Change summary and Tests.

Output format per PR: PR: Changelog:

... Good:
... Bad:
... Ugly:
... Questions or Assumptions:
... Change summary:
... Tests:
...

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 903

diff --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 && 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:

Missing changelog entry in packages/coding-agent/CHANGELOG.md. A changelog entry is required before merge. If the user decides to merge, I will add it.

Good:

Clean separation of concerns: before_bash_exec event gives extensions a unified hook for both tool-based and user (!/!!) bash execution
The uv.ts example demonstrates a practical use case (Python/uv interception) and serves as good documentation
BashExecBlockResult allows extensions to block bash commands with custom reasons
Environment variable manipulation via env override with undefined to remove keys is intuitive
The originalCommand field preserves the original command for logging/debugging while allowing command modification
The isError flag in tool_result allows extensions to convert successful tool results to errors (useful for policy enforcement)
Type exports are comprehensive: BashExecEvent, BashExecOverrides, BashExecSource, BeforeBashExecEvent, BeforeBashExecEventResult
Documentation in extensions.md and README.md is clear with good examples
Test update correctly mocks resolveShellExecutionOptions instead of getShellConfig to match the new implementation

Bad:

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:
1. If handler returns isError: true on success, throw error with content as message
2. If handler returns content on existing error without isError: true, the original error is rethrown (content ignored)
3. If handler returns content with isError: true on existing error, content becomes new error message
This 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:

The applyBeforeBashExecOverrides function in wrapper.ts is defined inside wrapToolWithExtensions, creating a new function definition on every call. This should be hoisted outside.
The PR description admits the approach of "abusing content to carry error messages" feels "somewhat consistent to how it works but a bit ugly." The semantic mismatch between "content" (for display) and "error message" (for Error constructor) creates a leaky abstraction.
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:

Should the executedCommand field be displayed in the bash execution UI component when it differs from command? Currently it's stored but invisible.
Is the intention that before_bash_exec handlers are chained? The implementation chains them (currentEvent = applyBashExecOverrides(currentEvent, overrideResult)), but this isn't documented.
Should blocking return a BashResult with an error message instead of throwing? The current behavior throws which may not provide the best UX in all cases.
The timeout override in before_bash_exec could allow extensions to extend or override timeouts. Is this intentional?

Change summary:

Adds before_bash_exec extension event fired before bash execution (both tool and user !/!!)
Extensions can rewrite command, cwd, env, shell, args, timeout, or block execution entirely
Adds executedCommand to session history when command is rewritten
Extends tool_result event to allow modifying error messages via isError: true + content
Adds resolveShellExecutionOptions helper for unified shell config resolution
Adds uv.ts example extension demonstrating Python/uv interception
Updates documentation in extensions.md and README.md

Tests:

Single test update: mocks resolveShellExecutionOptions instead of getShellConfig for spawn error test
No new tests for the before_bash_exec event handler
No tests for the tool_result error manipulation flow
No tests for chaining multiple before_bash_exec handlers
No tests for blocking execution with { block: true }

what is executeCommand supposed to do?

Looking at the PR diff, executedCommand (not executeCommand) is a new optional field added to BashExecutionMessage:

typescript

export interface BashExecutionMessage {	role: "bashExecution";	command: string;	executedCommand?: string;  // NEW	output: string;	exitCode: number | undefined;	// ...}

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:

typescript

this.recordBashResult(command, result, {    excludeFromContext: options?.excludeFromContext,    executedCommand: execCommand === command ? undefined : execCommand,});

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:

User runs: python script.py
before_bash_exec rewrites to: uv run python script.py
Session stores: command: "python script.py", executedCommand: "uv run python script.py"

This allows:

The UI to show the original command the user/model requested
Session history to record what actually ran (for debugging, audit, replay)

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):

typescript

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;

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):

typescript

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;}

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:

On success, isError: true in the result converts success to error
On error, you MUST return isError: true to replace the error message, otherwise your content is ignored
There's no way to convert an error into a success (swallow the error)

The !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>.

typescript

// 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    };});

// 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.

typescript

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 (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:

typescript

pi.on("tool_result", (event) => {    if (shouldAddHint(event)) {        return { content: [...event.content, { type: "text", text: hint }] };    }});

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 BashResult
before_bash_exec can modify command/env/cwd but not provide custom operations

Why 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:

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:

Single event: before_bash_exec - fires for all bash (tool + user), composable
Drop the isError return gymnastics - tool_result just modifies content, period
Remove executedCommand storage - if you need it, add an after_bash_exec event later
Don't extend BashOperations interface - that's a different feature (remote execution)

The event:

typescript

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 };

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.