fix(coding-agent): filtered brush-incompatible aliases from shell snapshot

brush-core's alias expander resolves aliases via
`value.split_ascii_whitespace()` (`crates/brush-core-vendored/src/interp.rs:1500`,
upstream brush issue reubeno/brush#57): each whitespace piece is dropped
into argv as-is, completely bypassing the shell parser. Any alias body
containing `(`, `)`, `|`, `&`, `;`, `<`, `>`, or `\`` therefore
turns the first piece into the command name, so Fedora's default
`alias which='(alias; declare -f) | /usr/bin/which …'` produces
`error: command not found: (alias;` for every `which` invocation.

The user's shell snapshot is generated by sourcing their real rc-file
under `/bin/bash` or `/bin/zsh` (so we can capture functions, options,
PATH) and then sourced by brush per-session. `sanitizeSnapshotForBrush`
now scans the emitted `alias -- NAME='VALUE'` lines after generation,
drops any whose decoded body contains those metacharacters, and rewrites
the file in place before caching. Compatible aliases (`ll='ls -l'`,
`gc='git --color=auto commit'`, embedded-quote `say='echo '\\''hi'\\'''`)
are preserved untouched; dropped names are logged at debug. brush then
falls through to whatever lives on `PATH`, which is what the user
expected when they ran `which` in the first place.

Covered by unit tests for the sanitizer (Fedora-which case, every
incompatible-metachar shape, every preserve case) and an integration
test that loads a poisoned snapshot and verifies `which sh` now exits
`0` with a real path.

Fixes #3234

packages/coding-agent/CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## [Unreleased]
 
+### Fixed
+
+- Filtered alias definitions brush's whitespace-only expander cannot execute (`(`, `)`, `|`, `&`, `;`, `<`, `>`, `` ` ``) from the bash-tool shell snapshot, so user rc-files containing compound aliases like Fedora's default `which='(alias; declare -f) | /usr/bin/which …'` no longer poison the brush session with `error: command not found: (alias;` ([#3234](https://github.com/can1357/oh-my-pi/issues/3234)).
+
 ## [16.1.14] - 2026-06-22
 
 ### Added

packages/coding-agent/src/utils/shell-snapshot.ts

@@ -8,11 +8,72 @@
 import * as fs from "node:fs";
 import * as os from "node:os";
 import * as path from "node:path";
-import { postmortem } from "@oh-my-pi/pi-utils";
+import { logger, postmortem } from "@oh-my-pi/pi-utils";
 
 const cachedSnapshotPaths = new Map<string, string>();
 const SNAPSHOT_TIMEOUT_MS = 2_000;
 
+/**
+ * Characters that force brush's primitive alias expander down a path it does
+ * not implement. brush-core resolves aliases via `value.split_ascii_whitespace()`
+ * (`crates/brush-core-vendored/src/interp.rs:1500`, tracking
+ * https://github.com/reubeno/brush/issues/57): the resulting pieces are dropped
+ * into argv verbatim instead of going through the shell parser. Any alias body
+ * containing subshells `(...)`, pipes `|`, redirections `<` `>`, separators
+ * `;` `&`, or command substitutions `` ` `` turns the first whitespace-split
+ * piece into the command name and produces `command not found: (alias;` style
+ * failures (issue #3234, Fedora's default `which` alias is the canonical case).
+ *
+ * Until brush implements proper alias parsing we drop these from the snapshot;
+ * brush then falls through to whatever lives on `PATH`, which is what the user
+ * actually expected when they invoked `which` / `ls` / etc.
+ */
+const BRUSH_INCOMPATIBLE_ALIAS_BODY = /[()|&;<>`]/;
+
+/** Matches `alias -- NAME='VALUE'` lines emitted by `generateSnapshotScript`. */
+const SNAPSHOT_ALIAS_LINE = /^alias -- ([^\s=]+)='(.*)'\s*$/;
+
+/**
+ * Strip alias definitions brush's whitespace-only expander cannot execute.
+ *
+ * Returns the rewritten snapshot plus the list of dropped alias names so the
+ * caller can surface them in the debug log.
+ */
+export function sanitizeSnapshotForBrush(content: string): { content: string; dropped: string[] } {
+	const dropped: string[] = [];
+	const lines = content.split("\n");
+	const out: string[] = [];
+	for (const line of lines) {
+		const m = line.match(SNAPSHOT_ALIAS_LINE);
+		if (m) {
+			// Decode the bash-quoting escape `'\''` → `'` so we test the real value.
+			const value = m[2].replace(/'\\''/g, "'");
+			if (BRUSH_INCOMPATIBLE_ALIAS_BODY.test(value)) {
+				dropped.push(m[1]);
+				continue;
+			}
+		}
+		out.push(line);
+	}
+	return { content: out.join("\n"), dropped };
+}
+
+/**
+ * Apply {@link sanitizeSnapshotForBrush} to the freshly generated snapshot
+ * file. Best-effort: I/O failures here must not poison `getOrCreateSnapshot`.
+ */
+function scrubSnapshotInPlace(snapshotPath: string): void {
+	try {
+		const raw = fs.readFileSync(snapshotPath, "utf8");
+		const { content, dropped } = sanitizeSnapshotForBrush(raw);
+		if (dropped.length === 0) return;
+		fs.writeFileSync(snapshotPath, content);
+		logger.debug("shell-snapshot: dropped brush-incompatible aliases", { dropped });
+	} catch (err) {
+		logger.debug("shell-snapshot: scrub failed", { err: String(err) });
+	}
+}
+
 function sanitizeSnapshotEnv(env: Record<string, string | undefined>): Record<string, string | undefined> {
 	const sanitized = { ...env };
 	delete sanitized.BASH_ENV;
@@ -169,6 +230,7 @@ export async function getOrCreateSnapshot(
 
 		await child.exited;
 		if (child.exitCode === 0 && fs.existsSync(snapshotPath)) {
+			scrubSnapshotInPlace(snapshotPath);
 			cachedSnapshotPaths.set(cacheKey, snapshotPath);
 			return snapshotPath;
 		}

packages/coding-agent/test/bash-executor.test.ts

@@ -797,6 +797,54 @@ exit 64
 		expect(result.output.trim()).toBe("snapshot_ok");
 	});
 
+	it("survives compound aliases from the user's shell snapshot (issue #3234)", async () => {
+		if (process.platform === "win32") return;
+		const bashPath = Bun.env.SHELL?.includes("bash") ? Bun.env.SHELL : "/bin/bash";
+		if (!fs.existsSync(bashPath)) return;
+
+		// Pre-seed a snapshot that mirrors Fedora's default `which` alias.
+		// Without the brush-compat scrub, brush's whitespace-only alias
+		// expander turns `(alias;` into the command name and `which` fails
+		// with `command not found: (alias;`. With the scrub, the broken
+		// alias is dropped and brush falls through to `$PATH`.
+		const snapshotPath = path.join(tempDir, "snapshot.sh");
+		fs.writeFileSync(
+			snapshotPath,
+			[
+				"unalias -a 2>/dev/null || true",
+				"alias -- which='(alias; declare -f) | /usr/bin/which --tty-only --read-alias --show-dot --show-tilde'",
+				"alias -- ll='ls -l'",
+				"",
+			].join("\n"),
+		);
+		const rawSnapshot = fs.readFileSync(snapshotPath, "utf8");
+		const { content: scrubbed, dropped } =
+			shellSnapshot.sanitizeSnapshotForBrush(rawSnapshot);
+		fs.writeFileSync(snapshotPath, scrubbed);
+		expect(dropped).toEqual(["which"]);
+		// Compatible aliases must still be installed in brush.
+		expect(scrubbed).toContain("alias -- ll='ls -l'");
+
+		vi.spyOn(Settings.prototype, "getShellConfig").mockReturnValue({
+			shell: bashPath,
+			args: ["-l", "-c"],
+			env: { PATH: Bun.env.PATH ?? "", HOME: Bun.env.HOME ?? tempDir },
+			prefix: undefined,
+		});
+		vi.spyOn(shellSnapshot, "getOrCreateSnapshot").mockResolvedValue(snapshotPath);
+
+		const result = await executeBash("which sh", {
+			cwd: tempDir,
+			timeout: 5000,
+			sessionKey: "brush-compound-alias-which",
+		});
+
+		expect(result.cancelled).toBe(false);
+		expect(result.exitCode).toBe(0);
+		expect(result.output).not.toContain("command not found");
+		expect(result.output.trim()).toMatch(/\/sh$/);
+	});
+
 	it("does not allow exec to replace the host", async () => {
 		const result = await executeBash("exec echo hi", { cwd: tempDir, timeout: 5000 });
 		expect(result.cancelled).toBe(false);

packages/coding-agent/test/shell-snapshot.test.ts

@@ -0,0 +1,80 @@
+import { describe, expect, it } from "bun:test";
+import { sanitizeSnapshotForBrush } from "@oh-my-pi/pi-coding-agent/utils/shell-snapshot";
+
+// `sanitizeSnapshotForBrush` is the snapshot-side mitigation for brush's
+// whitespace-only alias expander (`crates/brush-core-vendored/src/interp.rs:1500`,
+// brush issue reubeno/brush#57). Aliases whose body needs real shell parsing
+// must be dropped or brush turns the first whitespace piece into the command
+// name and the user sees `error: command not found: (alias;` (issue #3234).
+
+describe("sanitizeSnapshotForBrush", () => {
+	it("drops the Fedora `which` alias and reports the dropped name", () => {
+		const snapshot = [
+			"unalias -a 2>/dev/null || true",
+			"alias -- which='(alias; declare -f) | /usr/bin/which --tty-only --read-alias --show-dot --show-tilde'",
+			"alias -- ll='ls -l'",
+			"",
+		].join("\n");
+
+		const result = sanitizeSnapshotForBrush(snapshot);
+
+		expect(result.dropped).toEqual(["which"]);
+		expect(result.content).not.toContain("alias -- which=");
+		// Simple aliases must still pass through untouched.
+		expect(result.content).toContain("alias -- ll='ls -l'");
+	});
+
+	it.each([
+		["subshell", "alias -- a='(echo hi)'"],
+		["pipe", "alias -- a='cat /etc/hostname | head -n1'"],
+		["semicolon", "alias -- a='echo a; echo b'"],
+		["redirect-out", "alias -- a='tee >foo'"],
+		["redirect-in", "alias -- a='cat <foo'"],
+		["background", "alias -- a='sleep 1 &'"],
+		["command-substitution", "alias -- a='echo `date`'"],
+		// Embedded single-quote `'\''` → `'` decodes to a body with a literal
+		// single quote (and a pipe), so the filter still trips on the pipe.
+		["decoded-pipe", "alias -- a='cat '\\''one'\\'' | head'"],
+	])("drops aliases whose body needs shell parsing (%s)", (_label, line) => {
+		const result = sanitizeSnapshotForBrush(line + "\n");
+		expect(result.dropped).toEqual(["a"]);
+		expect(result.content).not.toContain("alias -- a=");
+	});
+
+	it.each([
+		["simple", "alias -- ll='ls -l'"],
+		["flag-with-equals", "alias -- gc='git --color=auto commit'"],
+		["multi-flag", "alias -- la='ls -lAh --group-directories-first'"],
+		// A plain single quote escape that decodes to a metachar-free body
+		// must survive — we only ban truly unparseable bodies.
+		["embedded-quote", "alias -- say='echo '\\''hello'\\'''"],
+	])("preserves aliases brush can handle by whitespace split (%s)", (_label, line) => {
+		const result = sanitizeSnapshotForBrush(line + "\n");
+		expect(result.dropped).toEqual([]);
+		expect(result.content).toContain(line);
+	});
+
+	it("leaves non-alias lines (functions, exports, unalias) untouched", () => {
+		const snapshot = [
+			"# Shell snapshot - generated by omp agent",
+			"unalias -a 2>/dev/null || true",
+			"my_fn () { echo hi; }",
+			"export PATH='/usr/bin:/bin'",
+			"shopt -s expand_aliases",
+			"alias -- which='(alias; declare -f) | /usr/bin/which'", // poisoned
+		].join("\n");
+
+		const result = sanitizeSnapshotForBrush(snapshot);
+
+		expect(result.dropped).toEqual(["which"]);
+		for (const keep of [
+			"# Shell snapshot - generated by omp agent",
+			"unalias -a 2>/dev/null || true",
+			"my_fn () { echo hi; }", // function body contains `;` but is not an alias line
+			"export PATH='/usr/bin:/bin'",
+			"shopt -s expand_aliases",
+		]) {
+			expect(result.content).toContain(keep);
+		}
+	});
+});