Merge PR #4455: feat(coding-agent): auto-load direnv environment into the bash session (@mattwilkinsonn)
# Conflicts: # packages/coding-agent/src/exec/bash-executor.ts # packages/coding-agent/test/bash-executor.test.ts
This commit is contained in:
@@ -1135,6 +1135,9 @@
|
||||
|
||||
- Made the statusline `git` segment jj-aware: in a Jujutsu repo it shows the nearest bookmark (falling back to the short change-id) instead of git's `detached` label or nothing, and working-copy change counts come from jj where there is no `.git` to read ([#3582](https://github.com/can1357/oh-my-pi/issues/3582))
|
||||
- Added `block`/`unblock` todo operations and a `blocked` status for tasks waiting on external input; blocked tasks stay visible in the todo HUD and summary but are excluded from the incomplete-todo stop reminder, and an optional blocker note records what the task is waiting for.
|
||||
### 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
|
||||
|
||||
|
||||
@@ -3347,6 +3347,29 @@ export const SETTINGS_SCHEMA = {
|
||||
},
|
||||
"bashInterceptor.patterns": { type: "array", default: DEFAULT_BASH_INTERCEPTOR_RULES },
|
||||
|
||||
"bash.direnv": {
|
||||
type: "enum",
|
||||
values: ["auto", "off"] as const,
|
||||
default: "auto",
|
||||
ui: {
|
||||
tab: "shell",
|
||||
group: "Bash",
|
||||
label: "direnv Auto-Load",
|
||||
description:
|
||||
"Auto-load (and auto-allow) a repo's direnv/devenv `.envrc` into the bash session so devenv tools and env vars are present without manual `direnv exec`",
|
||||
},
|
||||
},
|
||||
"bash.direnvLoadTimeoutMs": {
|
||||
type: "number",
|
||||
default: 30_000,
|
||||
ui: {
|
||||
tab: "shell",
|
||||
group: "Bash",
|
||||
label: "direnv Load Timeout (ms)",
|
||||
description:
|
||||
"Max wait for the first `direnv export` (a cold devenv shell can be slow); on timeout the session runs without the direnv env",
|
||||
},
|
||||
},
|
||||
// Shell output minimizer
|
||||
"shellMinimizer.enabled": {
|
||||
type: "boolean",
|
||||
|
||||
@@ -10,6 +10,7 @@ import { Settings, type ShellMinimizerSettings } from "../config/settings";
|
||||
import { OutputSink } from "../session/streaming-output";
|
||||
import { resolveOutputMaxColumns, resolveOutputSinkHeadBytes } from "../tools/output-meta";
|
||||
import { getOrCreateSnapshot } from "../utils/shell-snapshot";
|
||||
import { loadDirenvEnv } from "./direnv";
|
||||
import { buildNonInteractiveEnv } from "./non-interactive-env";
|
||||
|
||||
export interface BashExecutorOptions {
|
||||
@@ -56,6 +57,78 @@ export interface BashResult {
|
||||
workingDir?: string;
|
||||
}
|
||||
|
||||
/** POSIX-safe variable name — gates which direnv unsets we inject into the
|
||||
* command line, so a hostile `.envrc` can't smuggle shell syntax through
|
||||
* `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;
|
||||
/** Full direnv-load budget (`bash.direnvLoadTimeoutMs`). A positive
|
||||
* `callerTimeoutMs` clamps the effective load below this; `0`/undefined
|
||||
* leaves the full budget. */
|
||||
timeoutMs?: number;
|
||||
/** The caller's command deadline (ms). A positive value clamps the direnv
|
||||
* load so a cold `.envrc` can't outlast a short-timeout command; `0` or
|
||||
* undefined means "no caller clamp" — the load keeps its full `timeoutMs`
|
||||
* budget (a disabled command deadline is NOT a 0 ms load). Centralizing the
|
||||
* clamp here keeps every backend (executeBash, ACP terminal, PTY) on one
|
||||
* contract instead of each re-deriving it. */
|
||||
callerTimeoutMs?: 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);
|
||||
// A positive caller deadline clamps the direnv load below its full budget so
|
||||
// a cold `.envrc` can't outlast a short-timeout command; `0`/undefined means
|
||||
// "no caller clamp" (a disabled command deadline is not a 0 ms load). Every
|
||||
// backend routes through here, so the clamp lives in one place.
|
||||
const loadTimeoutMs =
|
||||
opts.callerTimeoutMs !== undefined && opts.callerTimeoutMs > 0
|
||||
? Math.min(opts.timeoutMs ?? opts.callerTimeoutMs, opts.callerTimeoutMs)
|
||||
: opts.timeoutMs;
|
||||
const direnvDiff =
|
||||
opts.direnvSetting === "off" ? null : await loadDirenvEnv(cwd, { timeoutMs: loadTimeoutMs, 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>>();
|
||||
@@ -279,15 +352,25 @@ export async function executeBash(command: string, options?: BashExecutorOptions
|
||||
const minimizer = buildMinimizerOptions(settings.getGroup("shellMinimizer"));
|
||||
|
||||
const commandCwd = resolveShellCwd(options?.cwd);
|
||||
const commandEnv = buildNonInteractiveEnv(options?.env);
|
||||
|
||||
// Apply command prefix if configured
|
||||
const prefixedCommand = 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: settings.get("bash.direnvLoadTimeoutMs"),
|
||||
callerTimeoutMs: options?.timeout,
|
||||
direnvSetting: settings.get("bash.direnv"),
|
||||
commandPrefix: prefix,
|
||||
});
|
||||
const commandEnv = buildNonInteractiveEnv(preflight.env);
|
||||
const runCdInPersistentShell = options?.useUserShell === true && !prefix && isPersistentShellCdCommand(command);
|
||||
const finalCommand =
|
||||
options?.useUserShell === true && !bashShell && !runCdInPersistentShell
|
||||
? buildUserShellCommand(shell, args, prefixedCommand)
|
||||
: prefixedCommand;
|
||||
? buildUserShellCommand(shell, args, preflight.command)
|
||||
: preflight.command;
|
||||
|
||||
// Create output sink for truncation and artifact handling
|
||||
const sink = new OutputSink({
|
||||
|
||||
@@ -0,0 +1,139 @@
|
||||
import * as fs from "node:fs/promises";
|
||||
import * as path from "node:path";
|
||||
import { $which, logger } from "@oh-my-pi/pi-utils";
|
||||
|
||||
/** Default cap on a single `direnv` invocation. The first export for a devenv
|
||||
* `.envrc` can build a shell; callers may raise this via `bash.direnvLoadTimeoutMs`. */
|
||||
export const DEFAULT_DIRENV_TIMEOUT_MS = 30_000;
|
||||
|
||||
/** Walk up from `startDir` to the nearest directory containing an `.envrc`. */
|
||||
export async function findEnvrc(startDir: string): Promise<string | null> {
|
||||
let dir = path.resolve(startDir);
|
||||
for (;;) {
|
||||
const candidate = path.join(dir, ".envrc");
|
||||
try {
|
||||
if ((await fs.stat(candidate)).isFile()) return candidate;
|
||||
} catch {
|
||||
// no .envrc here — keep walking up
|
||||
}
|
||||
const parent = path.dirname(dir);
|
||||
if (parent === dir) return null;
|
||||
dir = parent;
|
||||
}
|
||||
}
|
||||
|
||||
export interface DirenvExportDiff {
|
||||
/** Variables direnv sets to a concrete value. */
|
||||
set: Record<string, string>;
|
||||
/** Variables direnv removes (JSON `null`). */
|
||||
unset: string[];
|
||||
}
|
||||
|
||||
/** Parse `direnv export json` output (`{VAR: value|null}`) into set/unset halves. */
|
||||
export function parseDirenvExport(jsonText: string): DirenvExportDiff {
|
||||
const trimmed = jsonText.trim();
|
||||
if (trimmed.length === 0) return { set: {}, unset: [] };
|
||||
let parsed: unknown;
|
||||
try {
|
||||
parsed = JSON.parse(trimmed);
|
||||
} catch {
|
||||
return { set: {}, unset: [] };
|
||||
}
|
||||
const set: Record<string, string> = {};
|
||||
const unset: string[] = [];
|
||||
if (parsed && typeof parsed === "object") {
|
||||
for (const [key, value] of Object.entries(parsed as Record<string, unknown>)) {
|
||||
if (value === null) unset.push(key);
|
||||
else if (typeof value === "string") set[key] = value;
|
||||
}
|
||||
}
|
||||
return { set, unset };
|
||||
}
|
||||
|
||||
let direnvLookup: { bin: string | null } | undefined;
|
||||
function direnvBinary(): string | null {
|
||||
if (!direnvLookup) direnvLookup = { bin: $which("direnv") };
|
||||
return direnvLookup.bin;
|
||||
}
|
||||
|
||||
/** direnv computes its diff relative to the spawning env; strip any inherited
|
||||
* direnv state so it loads the target `.envrc` from a clean baseline. */
|
||||
function cleanSpawnEnv(): Record<string, string> {
|
||||
const out: Record<string, string> = {};
|
||||
for (const [key, value] of Object.entries(Bun.env)) {
|
||||
if (value !== undefined && !key.startsWith("DIRENV_")) out[key] = value;
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
async function runDirenv(
|
||||
bin: string,
|
||||
args: string[],
|
||||
cwd: string,
|
||||
timeoutMs: number,
|
||||
env: Record<string, string>,
|
||||
signal?: AbortSignal,
|
||||
): Promise<{ exitCode: number; stdout: string }> {
|
||||
// Bail on the caller's cancellation as well as the per-invocation cap so a
|
||||
// cold `.envrc` load can't outlive an aborted / short-timeout bash call.
|
||||
const abortSignal = signal
|
||||
? AbortSignal.any([signal, AbortSignal.timeout(timeoutMs)])
|
||||
: AbortSignal.timeout(timeoutMs);
|
||||
const proc = Bun.spawn([bin, ...args], {
|
||||
cwd,
|
||||
env,
|
||||
stdout: "pipe",
|
||||
stderr: "pipe",
|
||||
signal: abortSignal,
|
||||
});
|
||||
// Drain stdout AND stderr concurrently: a cold `use devenv`/Nix load emits
|
||||
// enough diagnostics to fill the stderr pipe and block the child forever if
|
||||
// only stdout is read (it would then wait out `timeoutMs`).
|
||||
const [stdout] = await Promise.all([
|
||||
new Response(proc.stdout as ReadableStream<Uint8Array>).text(),
|
||||
new Response(proc.stderr as ReadableStream<Uint8Array>).text(),
|
||||
]);
|
||||
const exitCode = await proc.exited;
|
||||
return { exitCode, stdout };
|
||||
}
|
||||
|
||||
/**
|
||||
* 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`.
|
||||
*/
|
||||
export async function loadDirenvEnv(
|
||||
cwd: string,
|
||||
opts?: { timeoutMs?: number; signal?: AbortSignal },
|
||||
): Promise<DirenvExportDiff | null> {
|
||||
const envrcPath = await findEnvrc(cwd);
|
||||
if (!envrcPath) return null;
|
||||
const bin = direnvBinary();
|
||||
if (!bin) return null;
|
||||
|
||||
const dir = path.dirname(envrcPath);
|
||||
const timeoutMs = opts?.timeoutMs ?? DEFAULT_DIRENV_TIMEOUT_MS;
|
||||
const env = cleanSpawnEnv();
|
||||
try {
|
||||
await runDirenv(bin, ["allow"], dir, timeoutMs, env, opts?.signal);
|
||||
const { exitCode, stdout } = await runDirenv(bin, ["export", "json"], dir, timeoutMs, env, opts?.signal);
|
||||
if (exitCode !== 0) {
|
||||
logger.warn("direnv export failed", { dir, exitCode });
|
||||
return null;
|
||||
}
|
||||
return parseDirenvExport(stdout);
|
||||
} catch (err) {
|
||||
logger.warn("direnv load failed", { dir, error: err instanceof Error ? err.message : String(err) });
|
||||
return null;
|
||||
}
|
||||
}
|
||||
@@ -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";
|
||||
@@ -1009,17 +1009,43 @@ 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.
|
||||
// `callerTimeoutMs` clamps the direnv load to a positive command timeout
|
||||
// (the backend's own timeout is installed only after this await), matching
|
||||
// the executeBash branch so a cold `.envrc` can't outlast a short call.
|
||||
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"),
|
||||
callerTimeoutMs: timeoutMs,
|
||||
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,
|
||||
});
|
||||
@@ -1193,15 +1219,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,10 +4,12 @@ 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 {
|
||||
applyDirenvPreflight,
|
||||
buildMinimizerOptions,
|
||||
executeBash,
|
||||
isPersistentShellCdCommand,
|
||||
} 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, ShellRunResult } from "@oh-my-pi/pi-natives";
|
||||
@@ -169,6 +171,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;
|
||||
@@ -1340,3 +1373,66 @@ describe("executeBash :async: background retention", () => {
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
describe("applyDirenvPreflight direnv-load clamp", () => {
|
||||
let tempDir: string;
|
||||
|
||||
beforeEach(() => {
|
||||
tempDir = makeTempDir();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
if (fs.existsSync(tempDir)) removeSyncWithRetries(tempDir);
|
||||
});
|
||||
|
||||
it("clamps the direnv load to a positive caller timeout below the full budget", async () => {
|
||||
// The clamp lives INSIDE the helper, so every backend that routes through
|
||||
// applyDirenvPreflight (executeBash, ACP terminal, PTY) inherits it. A
|
||||
// positive callerTimeoutMs smaller than the full load budget must win, so a
|
||||
// short-timeout command can't hand a cold `.envrc` the full 30s window.
|
||||
// Reverting the clamp to executeBash-only leaves the ACP/PTY backend
|
||||
// passing the full budget here, reddening this assertion.
|
||||
const spy = vi.spyOn(direnvModule, "loadDirenvEnv").mockResolvedValue(null);
|
||||
|
||||
await applyDirenvPreflight("true", tempDir, {
|
||||
direnvSetting: "auto",
|
||||
timeoutMs: 30_000,
|
||||
callerTimeoutMs: 5,
|
||||
});
|
||||
|
||||
expect(spy).toHaveBeenCalledTimes(1);
|
||||
expect(spy.mock.calls[0][1]?.timeoutMs).toBe(5);
|
||||
});
|
||||
|
||||
it("keeps the full direnv-load budget when the caller deadline is disabled (callerTimeoutMs: 0)", async () => {
|
||||
// A disabled command deadline (`0`) is NOT a 0 ms load — it means "no caller
|
||||
// clamp", so the load keeps its full `timeoutMs` budget. Collapsing it to 0
|
||||
// would make AbortSignal.timeout(0) abort instantly and silently drop the
|
||||
// repo's direnv env.
|
||||
const spy = vi.spyOn(direnvModule, "loadDirenvEnv").mockResolvedValue(null);
|
||||
|
||||
await applyDirenvPreflight("true", tempDir, {
|
||||
direnvSetting: "auto",
|
||||
timeoutMs: 30_000,
|
||||
callerTimeoutMs: 0,
|
||||
});
|
||||
|
||||
expect(spy).toHaveBeenCalledTimes(1);
|
||||
expect(spy.mock.calls[0][1]?.timeoutMs).toBe(30_000);
|
||||
});
|
||||
|
||||
it("keeps the full direnv-load budget when no caller deadline is supplied (callerTimeoutMs: undefined)", async () => {
|
||||
// An omitted caller deadline behaves like a disabled one: no clamp, full
|
||||
// budget. Guards the `!== undefined` half of the clamp condition.
|
||||
const spy = vi.spyOn(direnvModule, "loadDirenvEnv").mockResolvedValue(null);
|
||||
|
||||
await applyDirenvPreflight("true", tempDir, {
|
||||
direnvSetting: "auto",
|
||||
timeoutMs: 30_000,
|
||||
});
|
||||
|
||||
expect(spy).toHaveBeenCalledTimes(1);
|
||||
expect(spy.mock.calls[0][1]?.timeoutMs).toBe(30_000);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,289 @@
|
||||
import { afterEach, beforeEach, describe, expect, it } from "bun:test";
|
||||
import * as fs from "node:fs/promises";
|
||||
import * as path from "node:path";
|
||||
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";
|
||||
|
||||
/** Real-direnv cases need the binary on PATH; skip cleanly when it's absent so
|
||||
* the graceful-degradation code path (returns `null`) isn't asserted against. */
|
||||
const hasDirenv = $which("direnv") !== null;
|
||||
|
||||
const tmpDirs: TempDir[] = [];
|
||||
function tmp(): string {
|
||||
const dir = TempDir.createSync("@pi-direnv-");
|
||||
tmpDirs.push(dir);
|
||||
return dir.path();
|
||||
}
|
||||
|
||||
afterEach(async () => {
|
||||
for (const dir of tmpDirs.splice(0)) await dir.remove();
|
||||
});
|
||||
|
||||
describe("findEnvrc", () => {
|
||||
it("walks up to the nearest .envrc above the start dir", async () => {
|
||||
const root = tmp();
|
||||
await Bun.write(path.join(root, ".envrc"), "export A=1\n");
|
||||
const nested = path.join(root, "a", "b");
|
||||
await fs.mkdir(nested, { recursive: true });
|
||||
|
||||
expect(await findEnvrc(nested)).toBe(path.join(root, ".envrc"));
|
||||
});
|
||||
|
||||
it("prefers the nearest .envrc when monorepo dirs nest them", async () => {
|
||||
const root = tmp();
|
||||
await Bun.write(path.join(root, ".envrc"), "export A=1\n");
|
||||
const sub = path.join(root, "pkg");
|
||||
await fs.mkdir(sub, { recursive: true });
|
||||
await Bun.write(path.join(sub, ".envrc"), "export B=2\n");
|
||||
|
||||
expect(await findEnvrc(sub)).toBe(path.join(sub, ".envrc"));
|
||||
});
|
||||
|
||||
it("returns null when no .envrc exists up the tree", async () => {
|
||||
const nested = path.join(tmp(), "x", "y");
|
||||
await fs.mkdir(nested, { recursive: true });
|
||||
|
||||
expect(await findEnvrc(nested)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("parseDirenvExport", () => {
|
||||
it("splits set values from null unsets", () => {
|
||||
const out = parseDirenvExport('{"FOO":"bar","BAZ":null,"PATH":"/x:/y"}');
|
||||
|
||||
expect(out.set).toEqual({ FOO: "bar", PATH: "/x:/y" });
|
||||
expect(out.unset).toEqual(["BAZ"]);
|
||||
});
|
||||
|
||||
it("treats empty / whitespace output as no diff", () => {
|
||||
expect(parseDirenvExport("")).toEqual({ set: {}, unset: [] });
|
||||
expect(parseDirenvExport(" \n")).toEqual({ set: {}, unset: [] });
|
||||
});
|
||||
});
|
||||
|
||||
describe.skipIf(!hasDirenv)("loadDirenvEnv (real direnv, auto-allow)", () => {
|
||||
// `direnv allow` writes a trust entry into its data dir. Redirect HOME + the
|
||||
// XDG dirs to a throwaway tmp so real-direnv cases never leak allow state
|
||||
// into the dev/CI user's global direnv 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("auto-allows an untrusted .envrc and returns its exported vars + PATH additions", async () => {
|
||||
const root = tmp();
|
||||
await fs.mkdir(path.join(root, "bin"), { recursive: true });
|
||||
await Bun.write(path.join(root, ".envrc"), "export DIRENV_FEATURE_TEST=loaded\nPATH_add bin\n");
|
||||
|
||||
const diff = await loadDirenvEnv(root);
|
||||
|
||||
expect(diff?.set.DIRENV_FEATURE_TEST).toBe("loaded");
|
||||
expect(diff?.set.PATH).toContain(path.join(root, "bin"));
|
||||
});
|
||||
|
||||
it("reports variables a .envrc unsets", async () => {
|
||||
const root = tmp();
|
||||
await Bun.write(path.join(root, ".envrc"), "unset PI_DIRENV_UNSET_TEST\n");
|
||||
// direnv emits a JSON null for a var only when it was present in the
|
||||
// spawn env and the `.envrc` removes it, so seed it in the parent env.
|
||||
// (Avoid a `DIRENV_`-prefixed name — the loader strips those before spawn.)
|
||||
Bun.env.PI_DIRENV_UNSET_TEST = "present";
|
||||
try {
|
||||
const diff = await loadDirenvEnv(root);
|
||||
expect(diff?.set.PI_DIRENV_UNSET_TEST).toBeUndefined();
|
||||
expect(diff?.unset).toContain("PI_DIRENV_UNSET_TEST");
|
||||
} finally {
|
||||
delete Bun.env.PI_DIRENV_UNSET_TEST;
|
||||
}
|
||||
});
|
||||
|
||||
it("returns null when there is no .envrc to load", async () => {
|
||||
expect(await loadDirenvEnv(tmp())).toBeNull();
|
||||
});
|
||||
|
||||
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");
|
||||
|
||||
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)", () => {
|
||||
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("exposes direnv-loaded vars to the command while per-call env still wins", async () => {
|
||||
const root = tmp();
|
||||
await Bun.write(path.join(root, ".envrc"), "export DIRENV_WIRE_TEST=fromdirenv\nexport OVERRIDE_ME=fromdirenv\n");
|
||||
|
||||
const result = await executeBash('printf "%s|%s" "$DIRENV_WIRE_TEST" "$OVERRIDE_ME"', {
|
||||
cwd: root,
|
||||
env: { OVERRIDE_ME: "fromcaller" },
|
||||
});
|
||||
|
||||
expect(result.output).toContain("fromdirenv|fromcaller");
|
||||
});
|
||||
|
||||
it("removes variables the .envrc unsets from the command environment", async () => {
|
||||
const root = tmp();
|
||||
await Bun.write(path.join(root, ".envrc"), "unset PI_DIRENV_UNSET_E2E\n");
|
||||
// Inherited from the process env (as an OMP-provided var would be); the
|
||||
// caller does NOT re-supply it, so direnv's unset must strip it. `printenv`
|
||||
// exits non-zero and prints nothing when the name is genuinely absent. A
|
||||
// unique sessionKey forces a fresh shell that captures the var we just set.
|
||||
// (Avoid a `DIRENV_`-prefixed name — the loader strips those before spawn.)
|
||||
Bun.env.PI_DIRENV_UNSET_E2E = "leaked";
|
||||
try {
|
||||
const result = await executeBash('printenv PI_DIRENV_UNSET_E2E; printf "rc=%s" "$?"', {
|
||||
cwd: root,
|
||||
sessionKey: `direnv-unset-${Date.now()}`,
|
||||
});
|
||||
expect(result.output).toContain("rc=1");
|
||||
expect(result.output).not.toContain("leaked");
|
||||
} finally {
|
||||
delete Bun.env.PI_DIRENV_UNSET_E2E;
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
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);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user