feat(direnv): apply devenv env across all bash backends + always revalidate

Extract applyDirenvPreflight() so the ACP client terminal and PTY backends
get the same direnv/devenv overlay as executeBash (previously only the
one-shot path did). The helper is a pure (command, env) transform — merge
direnv's set under the caller's overlay, prepend a regex-gated unset -v for
removed vars — so interactive backends keep their own env shape (live TERM)
while executeBash still layers its non-interactive defaults on top. The three
dispatch branches are mutually exclusive, so no command is preflighted twice.

Also drop the content-hash export cache in loadDirenvEnv: always run
direnv export json and let direnv's own watch/mtime invalidation decide
freshness, so a changed watched file re-exports even when .envrc is unchanged.

Co-Authored-By: seal <noreply@sealedsecurity.com>
This commit is contained in:
Matt Wilkinson
2026-07-08 23:46:14 -04:00
parent 5ea0cc8419
commit 5941d797ba
6 changed files with 270 additions and 54 deletions
+3
View File
@@ -201,6 +201,9 @@
- Fixed ACP `terminal/create` sending the bash tool's full shell line in `command` with no `args`, which broke spec-conformant clients that spawn `command`+`args` directly (no implicit shell) — any command containing a space, pipe, `&&`, redirect, or `$(...)` failed with `ENOENT` and the agent silently degraded to read-only tools. The bash tool now wraps the shell line before calling `clientBridge.createTerminal`, reusing the same shell binary + args the local `bash-executor` resolves via `settings.getShellConfig()` (Git Bash / `bash.exe` on Windows, `$SHELL` with `sh` fallback on POSIX) so bash semantics — `$VAR`, `$(...)`, `source`, POSIX quoting, `-l` — are preserved on both platforms. ([#4333](https://github.com/can1357/oh-my-pi/issues/4333))
- Fixed inference worker subprocesses (TTS, STT, tiny-model, mnemopi embeddings) discarding stderr, which left every unexpected exit — most visibly the local Kokoro TTS worker's recurring `exit code 7` crash loop — undiagnosable from the parent's logs. `createWorkerSubprocess` now pipes stderr without starting a live read while the worker is idle, then drains the stream after `onExit`, emits captured lines to `logger.debug` under an `<exitLabel> stderr` message, and keeps the last 16 KiB in a bounded ring that gets appended to the `Error` surfaced through `onError`. The exit surface is synchronized with the post-exit drain via `SpawnedSubprocess.stderrDrained`, so the full native trace shows up on the `tts: worker error` line without reintroducing event-loop liveness from unref'd workers. ([#4324](https://github.com/can1357/oh-my-pi/issues/4324))
- Fixed Windows session tail loss after atomic compaction rewrites by fencing append writers during full-file replacement and gating the atomic publish on a `commitGuard` that the storage backend checks synchronously before rename, so a concurrent `flushSync` (Ctrl+C / session-exit) is not overwritten by the stale body serialized before it ran. Covers post-compaction prompts, tool results, title changes, and exit diagnostics on the current JSONL path ([#4338](https://github.com/can1357/oh-my-pi/issues/4338)).
### Fixed
- Extended the bash tool's direnv/devenv auto-loading to every backend: the ACP client terminal and the interactive PTY now receive the repo's direnv environment (variables set, and `unset -v` for variables the `.envrc` removes) — previously only the one-shot `executeBash` path did — via a shared preflight so all backends behave identically, with the caller's explicit env still winning. direnv loading also always re-runs `direnv export json` instead of serving a content-hashed cache, so a change to a `watch_file` target re-exports even when the `.envrc` text is unchanged (direnv's own watch invalidation is authoritative). ([#4455](https://github.com/can1357/oh-my-pi/issues/4455))
## [16.3.4] - 2026-07-03
+76 -28
View File
@@ -60,6 +60,58 @@ export interface BashResult {
* `unset`. `.envrc` never produces non-identifier names in practice. */
const SAFE_ENV_NAME = /^[A-Za-z_][A-Za-z0-9_]*$/;
export interface DirenvPreflightOptions {
/** Caller-supplied env overlay; these values win over direnv-provided ones. */
callerEnv?: Record<string, string>;
signal?: AbortSignal;
/** Cap on the direnv load; the caller's own timeout further clamps it. */
timeoutMs?: number;
/** `bash.direnv` setting — `"off"` skips the load entirely. */
direnvSetting: "auto" | "off";
/** Shell wrapper prefix (profiler/strace) to place *after* the unset prefix,
* matching `executeBash`'s ordering. Backends that apply their own shell
* wrapping (ACP `wrapShellLineForClientTerminal`) omit this. */
commandPrefix?: string | undefined;
}
/**
* Load the repo's direnv/devenv env and fold it into a `(command, env)` pair so
* every bash backend (one-shot `executeBash`, ACP client terminal, PTY) exposes
* the same devenv tools. Encapsulates: load the diff, merge `set` under the
* caller's overlay (caller wins), and prepend a regex-gated `unset -v` for
* variables the `.envrc` removes (skipping any the caller re-supplied).
*
* Returns the possibly-prefixed command plus the merged env, or the inputs
* unchanged (`env` = `callerEnv`) when direnv is off, absent, or has no `.envrc`.
* Pure transform: does NOT layer non-interactive env defaults — that stays the
* caller's job (so interactive PTY/ACP paths keep their own env shape).
*/
export async function applyDirenvPreflight(
command: string,
cwd: string,
opts: DirenvPreflightOptions,
): Promise<{ command: string; env: Record<string, string> | undefined }> {
const withPrefix = (line: string): string => (opts.commandPrefix ? `${opts.commandPrefix} ${line}` : line);
const direnvDiff =
opts.direnvSetting === "off"
? null
: await loadDirenvEnv(cwd, { timeoutMs: opts.timeoutMs, signal: opts.signal });
if (!direnvDiff) {
return { command: withPrefix(command), env: opts.callerEnv };
}
// The caller's explicit env still wins over direnv-provided values.
const mergedEnv = { ...direnvDiff.set, ...opts.callerEnv };
// direnv can also *remove* inherited variables (a `.envrc` doing
// `unset AWS_PROFILE`). An env overlay can only add/override, so prepend a
// real `unset` for those — unless the caller re-supplied the same var
// explicitly, in which case the caller wins.
const direnvUnsets = direnvDiff.unset.filter(
name => !(opts.callerEnv && name in opts.callerEnv) && SAFE_ENV_NAME.test(name),
);
const unsetPrefix = direnvUnsets.length > 0 ? `unset -v ${direnvUnsets.join(" ")}; ` : "";
return { command: `${unsetPrefix}${withPrefix(command)}`, env: mergedEnv };
}
const shellSessions = new Map<string, Shell>();
const brokenShellSessions = new Set<string>();
const shellSessionQuarantines = new Map<string, Promise<unknown>>();
@@ -221,36 +273,32 @@ export async function executeBash(command: string, options?: BashExecutorOptions
const minimizer = buildMinimizerOptions(settings.getGroup("shellMinimizer"));
const commandCwd = resolveShellCwd(options?.cwd);
// Load the repo's direnv/devenv env (cached per .envrc) so devenv tools land
// on PATH; the caller's explicit `env` still wins over direnv-provided values.
// Thread the caller's signal + timeout so an aborted / short-timeout call
// can't hang on a cold `.envrc` load before the abort listener is installed.
const direnvDiff =
settings.get("bash.direnv") === "off"
? null
: await loadDirenvEnv(commandCwd ?? process.cwd(), {
timeoutMs: Math.min(
settings.get("bash.direnvLoadTimeoutMs"),
options?.timeout ?? Number.POSITIVE_INFINITY,
),
signal: options?.signal,
});
const commandEnv = buildNonInteractiveEnv(direnvDiff ? { ...direnvDiff.set, ...options?.env } : options?.env);
// direnv can also *remove* inherited variables (a `.envrc` doing
// `unset AWS_PROFILE`). The per-command env overlay can only add/override, so
// prepend a real `unset` for those — unless the caller re-supplied the same
// var explicitly, in which case the caller wins.
const direnvUnsets = direnvDiff
? direnvDiff.unset.filter(name => !(options?.env && name in options.env) && SAFE_ENV_NAME.test(name))
: [];
const unsetPrefix = direnvUnsets.length > 0 ? `unset -v ${direnvUnsets.join(" ")}; ` : "";
// Apply command prefix if configured
const prefixedCommand = `${unsetPrefix}${prefix ? `${prefix} ${command}` : command}`;
// Fold the repo's direnv/devenv env into the command + env so devenv tools
// land on PATH; the caller's explicit `env` still wins. Thread the caller's
// signal + timeout so an aborted / short-timeout call can't hang on a cold
// `.envrc` load before the abort listener is installed. The helper applies
// the configured shell `prefix` after any `unset -v` it prepends.
const preflight = await applyDirenvPreflight(command, commandCwd ?? process.cwd(), {
callerEnv: options?.env,
signal: options?.signal,
timeoutMs: (() => {
const direnvBudget = settings.get("bash.direnvLoadTimeoutMs");
// A disabled command deadline (`timeout: 0`) means "no caller clamp",
// not a 0 ms direnv load — the load still gets its full budget. Only a
// positive caller timeout clamps the direnv window below that budget.
const callerDeadline = options?.timeout;
return callerDeadline !== undefined && callerDeadline > 0
? Math.min(direnvBudget, callerDeadline)
: direnvBudget;
})(),
direnvSetting: settings.get("bash.direnv"),
commandPrefix: prefix,
});
const commandEnv = buildNonInteractiveEnv(preflight.env);
const finalCommand =
options?.useUserShell === true && !bashShell
? buildUserShellCommand(shell, args, prefixedCommand)
: prefixedCommand;
? buildUserShellCommand(shell, args, preflight.command)
: preflight.command;
// Create output sink for truncation and artifact handling
const sink = new OutputSink({
+6 -17
View File
@@ -97,16 +97,17 @@ async function runDirenv(
return { exitCode, stdout };
}
/** Cache the parsed env per resolved `.envrc` + content hash, so the (possibly
* slow) first export is paid once and a changed `.envrc` re-loads. */
const exportCache = new Map<string, DirenvExportDiff>();
/**
* Resolve the nearest `.envrc` from `cwd`, auto-allow it, and return its
* `direnv export` diff (variables to set, and variables direnv removes).
* Returns `null` when there is no `.envrc`, `direnv` is not installed, or the
* export fails/times out.
*
* Always re-invokes `direnv export json` rather than serving a cached diff:
* direnv is fast once warm, and its own watch/mtime invalidation is the
* authoritative freshness signal (a content-hash cache here would go stale when
* a `watch_file` target changes without the `.envrc` text changing).
*
* Auto-allow is deliberate: OMP already runs the repository's own code, so its
* `.envrc` is trusted under the same model rather than forcing a manual
* `direnv allow`.
@@ -120,16 +121,6 @@ export async function loadDirenvEnv(
const bin = direnvBinary();
if (!bin) return null;
let cacheKey: string;
try {
const content = await fs.readFile(envrcPath);
cacheKey = `${envrcPath}\u0000${Bun.hash(content).toString(36)}`;
} catch {
return null;
}
const cached = exportCache.get(cacheKey);
if (cached) return cached;
const dir = path.dirname(envrcPath);
const timeoutMs = opts?.timeoutMs ?? DEFAULT_DIRENV_TIMEOUT_MS;
const env = cleanSpawnEnv();
@@ -140,9 +131,7 @@ export async function loadDirenvEnv(
logger.warn("direnv export failed", { dir, exitCode });
return null;
}
const diff = parseDirenvExport(stdout);
exportCache.set(cacheKey, diff);
return diff;
return parseDirenvExport(stdout);
} catch (err) {
logger.warn("direnv load failed", { dir, error: err instanceof Error ? err.message : String(err) });
return null;
+35 -7
View File
@@ -10,7 +10,7 @@ import type { Component } from "@oh-my-pi/pi-tui";
import { ImageProtocol, TERMINAL } from "@oh-my-pi/pi-tui";
import { getProjectDir, isEnoent, logger, prompt } from "@oh-my-pi/pi-utils";
import { type } from "arktype";
import { type BashResult, executeBash } from "../exec/bash-executor";
import { applyDirenvPreflight, type BashResult, executeBash } from "../exec/bash-executor";
import type { RenderResultOptions } from "../extensibility/custom-tools/types";
import { InternalUrlRouter } from "../internal-urls";
import { truncateToVisualLines } from "../modes/components/visual-truncate";
@@ -882,17 +882,39 @@ export class BashTool implements AgentTool<typeof bashSchemaBase | typeof bashSc
});
}
// Fold direnv/devenv env into (command, env) ONCE for the two backends
// that bypass `executeBash` — the ACP client terminal and the PTY. The
// `executeBash` branch below is intentionally excluded: it runs its own
// preflight internally, so routing the pre-applied command there too
// would double-apply the unset prefix and re-merge the env. No
// `commandPrefix` here: ACP applies the shell prefix via
// `wrapShellLineForClientTerminal`, and the PTY path never wrapped one.
const backendPreflight =
(clientBridge?.capabilities.terminal && clientBridge.createTerminal && !pty) ||
canUseInteractiveBashPty(pty, ctx)
? await applyDirenvPreflight(command, commandCwd, {
callerEnv: resolvedEnv,
signal,
timeoutMs: this.session.settings.get("bash.direnvLoadTimeoutMs"),
direnvSetting: this.session.settings.get("bash.direnv"),
})
: undefined;
// Route through the client terminal when the client advertises the terminal capability.
// Skip when pty=true (PTY needs the local terminal UI).
if (clientBridge?.capabilities.terminal && clientBridge.createTerminal && !pty) {
const bridgeWallTimeStart = performance.now();
const shellSpawn = wrapShellLineForClientTerminal(command, this.session.settings.getShellConfig());
// direnv-transformed command (carries any `unset -v` prefix) + merged
// env; falls back to the raw command/env when direnv is off/absent.
const bridgeCommand = backendPreflight?.command ?? command;
const bridgeEnv = backendPreflight?.env ?? resolvedEnv;
const shellSpawn = wrapShellLineForClientTerminal(bridgeCommand, this.session.settings.getShellConfig());
const handle = await clientBridge.createTerminal({
command: shellSpawn.command,
args: shellSpawn.args,
cwd: commandCwd,
env: resolvedEnv
? Object.entries(resolvedEnv).map(([name, value]) => ({ name, value: value as string }))
env: bridgeEnv
? Object.entries(bridgeEnv).map(([name, value]) => ({ name, value: value as string }))
: undefined,
outputByteLimit: DEFAULT_MAX_BYTES,
});
@@ -1070,15 +1092,21 @@ export class BashTool implements AgentTool<typeof bashSchemaBase | typeof bashSc
const wallTimeStart = performance.now();
const result: BashResult | BashInteractiveResult = interactiveUi
? await runInteractiveBashPty(interactiveUi, {
command,
// PTY bypasses executeBash, so feed it the direnv-transformed
// command + merged env (backendPreflight is defined whenever this
// branch runs, since both gate on canUseInteractiveBashPty).
command: backendPreflight?.command ?? command,
cwd: commandCwd,
timeoutMs,
signal,
env: resolvedEnv,
env: backendPreflight?.env ?? resolvedEnv,
artifactPath,
artifactId,
})
: await executeBash(command, {
: // executeBash runs its OWN direnv preflight internally — pass the RAW
// command + resolvedEnv here so the unset prefix / env merge is not
// applied twice.
await executeBash(command, {
cwd: commandCwd,
sessionKey: this.session.getSessionId?.() ?? undefined,
timeout: timeoutMs ?? 0,
@@ -4,6 +4,7 @@ import * as os from "node:os";
import * as path from "node:path";
import { resetSettingsForTest, Settings, type ShellMinimizerSettings } from "@oh-my-pi/pi-coding-agent/config/settings";
import { buildMinimizerOptions, executeBash } from "@oh-my-pi/pi-coding-agent/exec/bash-executor";
import * as direnvModule from "@oh-my-pi/pi-coding-agent/exec/direnv";
import { DEFAULT_MAX_BYTES } from "@oh-my-pi/pi-coding-agent/session/streaming-output";
import * as shellSnapshot from "@oh-my-pi/pi-coding-agent/utils/shell-snapshot";
import type { Shell } from "@oh-my-pi/pi-natives";
@@ -137,6 +138,37 @@ describe("executeBash", () => {
expect(result.output.trim()).toBe(tempDir);
});
it("passes the full direnv-load budget when the command deadline is disabled (timeout: 0)", async () => {
// A disabled command deadline (`timeout: 0`) must NOT collapse the direnv
// export window to 0 ms — that would make AbortSignal.timeout(0) abort the
// load instantly, silently dropping the repo's direnv env. The load keeps
// its full `bash.direnvLoadTimeoutMs` budget. Spying on loadDirenvEnv both
// captures the timeoutMs and short-circuits real direnv (null diff = no-op).
const budget = (await Settings.init()).get("bash.direnvLoadTimeoutMs");
const spy = vi.spyOn(direnvModule, "loadDirenvEnv").mockResolvedValue(null);
await executeBash("true", { cwd: tempDir, timeout: 0 });
expect(spy).toHaveBeenCalledTimes(1);
expect(spy.mock.calls[0][1]?.timeoutMs).toBe(budget);
expect(spy.mock.calls[0][1]?.timeoutMs).not.toBe(0);
});
it("clamps the direnv-load budget to a positive command timeout smaller than it", async () => {
// A positive caller timeout below the budget DOES clamp the direnv window,
// proving the fix only relaxes the `timeout: 0` case and did not disable
// clamping wholesale. Setting and options.timeout are both milliseconds.
const budget = (await Settings.init()).get("bash.direnvLoadTimeoutMs");
const callerTimeout = 5;
expect(callerTimeout).toBeLessThan(budget);
const spy = vi.spyOn(direnvModule, "loadDirenvEnv").mockResolvedValue(null);
await executeBash("true", { cwd: tempDir, timeout: callerTimeout });
expect(spy).toHaveBeenCalledTimes(1);
expect(spy.mock.calls[0][1]?.timeoutMs).toBe(Math.min(budget, callerTimeout));
});
it("honors symlinked cwd requests in persistent shells", async () => {
if (process.platform === "win32") {
return;
+118 -2
View File
@@ -1,7 +1,7 @@
import { afterEach, beforeEach, describe, expect, it } from "bun:test";
import * as fs from "node:fs/promises";
import * as path from "node:path";
import { executeBash } from "@oh-my-pi/pi-coding-agent/exec/bash-executor";
import { applyDirenvPreflight, executeBash } from "@oh-my-pi/pi-coding-agent/exec/bash-executor";
import { findEnvrc, loadDirenvEnv, parseDirenvExport } from "@oh-my-pi/pi-coding-agent/exec/direnv";
import { $which, TempDir } from "@oh-my-pi/pi-utils";
@@ -112,7 +112,7 @@ describe.skipIf(!hasDirenv)("loadDirenvEnv (real direnv, auto-allow)", () => {
expect(await loadDirenvEnv(tmp())).toBeNull();
});
it("re-loads when the .envrc content changes (cache keyed by content)", async () => {
it("re-exports when the .envrc content changes (no stale cache)", async () => {
const root = tmp();
await Bun.write(path.join(root, ".envrc"), "export DIRENV_CACHE_TEST=one\n");
expect((await loadDirenvEnv(root))?.set.DIRENV_CACHE_TEST).toBe("one");
@@ -120,6 +120,24 @@ describe.skipIf(!hasDirenv)("loadDirenvEnv (real direnv, auto-allow)", () => {
await Bun.write(path.join(root, ".envrc"), "export DIRENV_CACHE_TEST=two\n");
expect((await loadDirenvEnv(root))?.set.DIRENV_CACHE_TEST).toBe("two");
});
it("always re-invokes direnv so a changed watched file re-exports even when .envrc text is unchanged", async () => {
// direnv's own `watch_file` invalidation — not a content hash of the
// `.envrc` — is the freshness authority. The `.envrc` bytes never change
// here; only the watched file's contents do. A content-hash early-return
// (the old behavior) would serve the stale first value; always running
// `direnv export json` picks up the new one.
const root = tmp();
await Bun.write(path.join(root, "watched.env"), "one\n");
await Bun.write(
path.join(root, ".envrc"),
'watch_file watched.env\nexport DIRENV_WATCH_TEST="$(cat watched.env)"\n',
);
expect((await loadDirenvEnv(root))?.set.DIRENV_WATCH_TEST).toBe("one");
await Bun.write(path.join(root, "watched.env"), "two\n");
expect((await loadDirenvEnv(root))?.set.DIRENV_WATCH_TEST).toBe("two");
});
});
describe.skipIf(!hasDirenv)("bash executor direnv wiring (end-to-end)", () => {
@@ -171,3 +189,101 @@ describe.skipIf(!hasDirenv)("bash executor direnv wiring (end-to-end)", () => {
}
});
});
describe.skipIf(!hasDirenv)("applyDirenvPreflight (shared all-backends preflight)", () => {
// This helper is what the ACP client-terminal and PTY backends call so they
// expose the same devenv env as `executeBash`. Assert its core contract:
// set-merge with caller-wins, the regex-gated `unset -v` prefix, and the
// off/absent passthrough. HOME/XDG isolation mirrors the loader cases so
// `direnv allow` never touches the dev/CI user's global store.
const savedEnv: Record<string, string | undefined> = {};
beforeEach(() => {
const home = tmp();
for (const key of ["HOME", "XDG_DATA_HOME", "XDG_CONFIG_HOME", "XDG_CACHE_HOME"]) {
savedEnv[key] = Bun.env[key];
Bun.env[key] = path.join(home, key.toLowerCase());
}
});
afterEach(() => {
for (const [key, value] of Object.entries(savedEnv)) {
if (value === undefined) delete Bun.env[key];
else Bun.env[key] = value;
}
});
it("merges direnv set vars into env while the caller's env wins", async () => {
const root = tmp();
await Bun.write(path.join(root, ".envrc"), "export DIRENV_PF_SET=fromdirenv\nexport OVERRIDE_ME=fromdirenv\n");
const { command, env } = await applyDirenvPreflight("echo hi", root, {
callerEnv: { OVERRIDE_ME: "fromcaller" },
direnvSetting: "auto",
});
expect(env?.DIRENV_PF_SET).toBe("fromdirenv");
expect(env?.OVERRIDE_ME).toBe("fromcaller");
// No unset in this .envrc, so the command is returned unprefixed.
expect(command).toBe("echo hi");
});
it("prepends a regex-gated `unset -v` for vars the .envrc removes, skipping caller-resupplied names", async () => {
const root = tmp();
await Bun.write(path.join(root, ".envrc"), "unset PI_PF_UNSET_A\nunset PI_PF_UNSET_B\n");
Bun.env.PI_PF_UNSET_A = "present";
Bun.env.PI_PF_UNSET_B = "present";
try {
const { command } = await applyDirenvPreflight("run-it", root, {
// Caller re-supplies B, so its unset must be skipped (caller wins).
callerEnv: { PI_PF_UNSET_B: "kept" },
direnvSetting: "auto",
});
expect(command).toContain("unset -v PI_PF_UNSET_A");
expect(command).not.toContain("PI_PF_UNSET_B");
expect(command.endsWith("run-it")).toBe(true);
} finally {
delete Bun.env.PI_PF_UNSET_A;
delete Bun.env.PI_PF_UNSET_B;
}
});
it("applies the shell commandPrefix after the unset prefix", async () => {
const root = tmp();
await Bun.write(path.join(root, ".envrc"), "unset PI_PF_ORDER\n");
Bun.env.PI_PF_ORDER = "present";
try {
const { command } = await applyDirenvPreflight("payload", root, {
direnvSetting: "auto",
commandPrefix: "strace -f",
});
// Ordering must be: `unset -v NAME; <prefix> <command>`.
expect(command).toBe("unset -v PI_PF_ORDER; strace -f payload");
} finally {
delete Bun.env.PI_PF_ORDER;
}
});
it("returns the command + caller env unchanged when direnv is off", async () => {
const root = tmp();
await Bun.write(path.join(root, ".envrc"), "export DIRENV_PF_OFF=loaded\n");
const callerEnv = { FOO: "bar" };
const { command, env } = await applyDirenvPreflight("noop", root, {
callerEnv,
direnvSetting: "off",
});
expect(command).toBe("noop");
expect(env).toBe(callerEnv);
});
it("returns the command + caller env unchanged when no .envrc exists", async () => {
const callerEnv = { FOO: "bar" };
const { command, env } = await applyDirenvPreflight("noop", tmp(), {
callerEnv,
direnvSetting: "auto",
});
expect(command).toBe("noop");
expect(env).toBe(callerEnv);
});
});