From 1284a2631ef07995d4e138a8a2952752c034527c Mon Sep 17 00:00:00 2001 From: Lingrui Mei Date: Sat, 4 Jul 2026 15:47:00 +0800 Subject: [PATCH] bash: image_env() util + subprocess.run env parity (drop clean_env flag) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #151 per maintainer review: a boolean flag on run/run_stream adds cognitive cost to a tool that should read like subprocess.run, and the mechanism to pick an environment already exists — you pass env=. So instead of a mode flag: - bash.run/run_stream env= now follows subprocess.run exactly: omit it to inherit the worker's environment (right for bundle tools — the claude CLI, bundled rg), pass a dict to REPLACE it wholesale. No merge, no flag. (No production caller passed a partial env expecting a merge — only tests, updated to the replace idiom.) - image_env() is the extracted helper the swebench scorer hand-rolled: get_env_without_agentix() plus BASH_ENV -> ~/.bashrc (derived from the RESULT env's HOME, so a stripped-and-restored or overridden HOME picks the right rc file). Canonical name: agentix.bash.image_env, re-exported next to run(); implemented in agentix.runtime.shared.env because that module owns the ADDED/SAVED contract and a cross-plugin import would not resolve statically in the pkgutil namespace layout. swebench's score.py now calls it instead of its private copy. - The shell binary follows the env's PATH like everything else the command resolves: the default worker env lists the bundled bash first (behavior unchanged for bundle calls), while an image_env() PATH selects the image's own bash — its restored LD_PRELOAD targets that binary, not the Nix one. image_env() reads the calling process's live environment, so it is only meaningful inside the sandbox; the docs show the host-side recipe (env = await c.remote(image_env)) and warn that evaluating it host-side would ship the host's environment into the container. Co-Authored-By: Claude Fable 5 --- agentix/runtime/shared/env.py | 24 +++++ plugins/datasets/swebench/src/score.py | 15 +-- .../runtime-basic/agentix/bash/__init__.py | 82 +++++++------- .../runtime-basic/tests/test_primitives.py | 100 ++++++++++++++---- tests/runtime/test_env.py | 21 ++-- 5 files changed, 166 insertions(+), 76 deletions(-) diff --git a/agentix/runtime/shared/env.py b/agentix/runtime/shared/env.py index 4a9acc5..3743854 100644 --- a/agentix/runtime/shared/env.py +++ b/agentix/runtime/shared/env.py @@ -144,6 +144,29 @@ def _target_var(tracking_var: str) -> str | None: return name or None +def image_env(extra: Mapping[str, str] | None = None) -> dict[str, str]: + """The task image's own environment, ready to pass as a subprocess env. + + :func:`get_env_without_agentix` plus a shell convenience: if the image + ships a ``~/.bashrc`` (per the RESULT env's ``HOME``) and nothing set + ``BASH_ENV``, point ``BASH_ENV`` at it so a non-interactive ``bash -c`` + sources the image's shell setup. Canonically re-exported as + ``agentix.bash.image_env`` — implemented here because this module owns + the ``AGENTIX_ADDED_*``/``AGENTIX_SAVED_*`` contract and is importable + from every plugin without a cross-plugin dependency. + + Reads the calling process's live environment, so it is only meaningful + INSIDE the sandbox; a host driving the sandbox fetches it over the wire + (``env = await c.remote(image_env)``) rather than evaluating it host-side. + """ + env = get_env_without_agentix(extra) + home = env.get("HOME") or os.path.expanduser("~") + bashrc = os.path.join(home, ".bashrc") + if "BASH_ENV" not in env and os.path.isfile(bashrc): + env["BASH_ENV"] = bashrc + return env + + def get_env_without_agentix( extra: Mapping[str, str] | None = None, *, @@ -207,4 +230,5 @@ def get_env_without_agentix( "BUNDLE_RUNTIME_VENV", "BUNDLE_RUNTIME_VENV_BIN", "get_env_without_agentix", + "image_env", ] diff --git a/plugins/datasets/swebench/src/score.py b/plugins/datasets/swebench/src/score.py index 8e10e54..c96a093 100644 --- a/plugins/datasets/swebench/src/score.py +++ b/plugins/datasets/swebench/src/score.py @@ -260,13 +260,14 @@ async def _run(command: str, workdir: str, env: dict[str, str], timeout: float, def _get_env() -> dict[str, str]: - from agentix.runtime.shared.env import get_env_without_agentix - - env = get_env_without_agentix() - bashrc = Path.home() / ".bashrc" - if bashrc.is_file() and "BASH_ENV" not in env: - env["BASH_ENV"] = str(bashrc) - return env + # The image's own environment (bundle paths subtracted, stripped vars + # restored, BASH_ENV pointed at ~/.bashrc). Imported from the core + # contract module — same symbol as `agentix.bash.image_env`, but a + # cross-plugin import would not resolve statically in the pkgutil + # namespace layout. + from agentix.runtime.shared.env import image_env + + return image_env() def _apply_export(command: str, env: dict[str, str]) -> bool: diff --git a/plugins/runtime-basic/agentix/bash/__init__.py b/plugins/runtime-basic/agentix/bash/__init__.py index d1d88e8..641e835 100644 --- a/plugins/runtime-basic/agentix/bash/__init__.py +++ b/plugins/runtime-basic/agentix/bash/__init__.py @@ -20,6 +20,16 @@ async functions, dataclasses (`BashResult`, `BashStdout`, …) coexist as types callers can import. The framework's discovery picks the async functions; types and constants are just regular Python imports. + +`env` follows `subprocess.run` semantics: omit it to inherit the worker's +environment (right for bundle tools — the claude CLI, bundled `rg`), or pass +a full dict to replace it. `image_env()` builds the task image's own +environment (bundle path additions subtracted, stripped vars restored) so a +task command resolves the image's toolchain rather than the bundle venv. +It must be evaluated INSIDE the sandbox — sandbox-side code calls +`run(cmd, env=image_env())` directly; a host fetches it over the wire first +(`env = await c.remote(image_env)`) rather than evaluating it host-side, +which would ship the host's environment into the container. """ from __future__ import annotations @@ -33,40 +43,40 @@ from pydantic import Field -from agentix.runtime.shared.env import BUNDLE_RUNTIME_BASH, get_env_without_agentix +from agentix.runtime.shared.env import BUNDLE_RUNTIME_BASH +from agentix.runtime.shared.env import image_env as image_env +# `image_env` is re-exported here — its canonical home for users — so the +# one-stop recipe for running a task command in the image's own environment +# lives right next to `run`. It reads the calling process's live environment, +# so it only means something INSIDE the sandbox: sandbox-side code calls +# `run(cmd, env=image_env())` directly; a host fetches it over the wire first +# (`env = await c.remote(image_env)`) — evaluating it host-side would ship +# the host's environment into the container. -def _clean_env(extra: dict[str, str] | None, clean: bool = False) -> dict[str, str]: - """Build a subprocess env, then apply caller overrides last. - Default: the worker's own environment — right for runtime tools (the - claude CLI, bundled rg, ...) that live under /nix. ``clean=True``: the - task IMAGE's environment (bundle path additions subtracted, stripped vars - restored) — right for task commands that must resolve the image's own - toolchain instead of the bundle venv. - """ - env = get_env_without_agentix() if clean else dict(os.environ) - if extra: - env.update(extra) - return env +def _build_env(env: dict[str, str] | None) -> dict[str, str]: + """subprocess.run parity: ``env=None`` inherits the worker's environment; + an explicit dict REPLACES it wholesale (no merge). Callers wanting + inherit-plus-tweak pass ``{**image_env(), "X": "1"}`` themselves.""" + return dict(os.environ) if env is None else dict(env) -def _shell_executable(executable: str | None, env: dict[str, str], clean: bool = False) -> str: +def _shell_executable(executable: str | None, env: dict[str, str]) -> str: + """The shell follows the env's PATH, like everything else the command + resolves: the default worker env lists the bundled bash first, so bundle + calls keep the known-good shell, while an ``image_env()`` PATH selects + the image's own bash — whose restored vars (LD_PRELOAD, ...) target that + binary, not the Nix one. Bundled bash and /bin/bash are fallbacks for + envs with no bash on PATH.""" if executable: return shutil.which(executable, path=env.get("PATH")) or executable - if clean: - # A clean-env command belongs to the image's world entirely — its - # restored vars (LD_PRELOAD, ...) target the image's own bash, not - # the Nix one. Fall back to the bundled bash only when the image has - # none at all. - image_bash = shutil.which("bash", path=env.get("PATH")) - if image_bash: - return image_bash - if os.access("/bin/bash", os.X_OK): - return "/bin/bash" + found = shutil.which("bash", path=env.get("PATH")) + if found: + return found if os.access(BUNDLE_RUNTIME_BASH, os.X_OK): return BUNDLE_RUNTIME_BASH - return shutil.which("bash", path=env.get("PATH")) or "/bin/bash" + return "/bin/bash" async def _read_capped(stream: asyncio.StreamReader, limit: int) -> str: @@ -162,23 +172,21 @@ async def run( timeout: float | None = None, max_output: int = 10 * 1024 * 1024, executable: str | None = None, - clean_env: bool = False, ) -> BashResult: """Run a shell command in the sandbox and return its captured output. - ``clean_env=True`` runs the command in the task image's environment - (Agentix's recorded path additions subtracted, stripped vars restored) - instead of the bundle-polluted worker environment; ``env`` overrides win - last either way. + Like ``subprocess.run``: ``env=None`` inherits the worker's environment; + an explicit ``env`` dict is the child's environment verbatim. To run in + the task image's own world (not the bundle's), pass ``env=image_env()``. """ - sub_env = _clean_env(env, clean=clean_env) + sub_env = _build_env(env) proc = await asyncio.create_subprocess_shell( command, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, cwd=cwd, env=sub_env, - executable=_shell_executable(executable, sub_env, clean=clean_env), + executable=_shell_executable(executable, sub_env), ) assert proc.stdout is not None and proc.stderr is not None stdout_task = asyncio.create_task(_read_capped(proc.stdout, max_output)) @@ -209,22 +217,22 @@ async def run_stream( env: dict[str, str] | None = None, timeout: float | None = None, executable: str | None = None, - clean_env: bool = False, ) -> AsyncIterator[BashEvent]: """Run a shell command, yielding events as the subprocess emits them. Terminates with a single `BashExit` event on normal completion or - a single `BashError` event on timeout / wire-level failure. - ``clean_env`` selects the task image's environment, as in `run`. + a single `BashError` event on timeout / wire-level failure. ``env`` + follows subprocess.run semantics, as in `run` (pass ``env=image_env()`` + to run in the task image's environment). """ - sub_env = _clean_env(env, clean=clean_env) + sub_env = _build_env(env) proc = await asyncio.create_subprocess_shell( command, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, cwd=cwd, env=sub_env, - executable=_shell_executable(executable, sub_env, clean=clean_env), + executable=_shell_executable(executable, sub_env), ) async def _pump(stream, tag, queue): diff --git a/plugins/runtime-basic/tests/test_primitives.py b/plugins/runtime-basic/tests/test_primitives.py index 3ebb70c..0f5cb24 100644 --- a/plugins/runtime-basic/tests/test_primitives.py +++ b/plugins/runtime-basic/tests/test_primitives.py @@ -56,7 +56,7 @@ async def test_bash_run_honors_bash_env(tmp_path: Path): result = await bash.run( "printf '%s' \"$FROM_BASH_ENV\"", - env={"BASH_ENV": str(bash_env)}, + env={**os.environ, "BASH_ENV": str(bash_env)}, ) assert result.exit_code == 0 @@ -116,42 +116,66 @@ async def test_bash_run_stream_can_use_explicit_executable(tmp_path: Path): @pytest.mark.asyncio -async def test_bash_run_clean_env_runs_in_the_image_environment(monkeypatch): - """clean_env=True subtracts the bundle's recorded PATH additions and - restores stripped vars, so the command resolves the image's toolchain.""" +async def test_bash_run_none_env_inherits_worker_environment(monkeypatch): + """subprocess.run parity: no env → inherit the worker's environment.""" + monkeypatch.setenv("AGENTIX_TEST_MARKER", "inherited") + result = await bash.run(command='printf %s "$AGENTIX_TEST_MARKER"') + assert result.stdout == "inherited" + + +@pytest.mark.asyncio +async def test_bash_run_env_dict_replaces_the_environment(monkeypatch): + """subprocess.run parity: an explicit env dict REPLACES the environment; + a worker var not in it is gone from the child (no merge).""" + monkeypatch.setenv("AGENTIX_TEST_MARKER", "should-not-leak") + result = await bash.run( + command='printf %s "${AGENTIX_TEST_MARKER-absent}"', + env={"PATH": os.environ.get("PATH", "/usr/bin")}, + ) + assert result.stdout == "absent" + + +def test_image_env_subtracts_additions_and_restores_saved(monkeypatch): + """The task-image env util: recorded bundle PATH additions subtracted, + stripped vars restored from their AGENTIX_SAVED_* snapshot.""" + from agentix.bash import image_env + + from agentix.runtime.shared.env import AGENTIX_ADDED_PATH + + monkeypatch.setenv("PATH", os.pathsep.join(["/nix/runtime/bin", "/usr/bin"])) + monkeypatch.setenv(AGENTIX_ADDED_PATH, "/nix/runtime/bin") + monkeypatch.setenv("AGENTIX_SAVED_PYTHONPATH", "/testbed/src") + monkeypatch.delenv("PYTHONPATH", raising=False) + + env = image_env() + assert env["PATH"] == "/usr/bin" + assert env["PYTHONPATH"] == "/testbed/src" + assert AGENTIX_ADDED_PATH not in env + assert "AGENTIX_SAVED_PYTHONPATH" not in env + + +@pytest.mark.asyncio +async def test_bash_run_with_image_env_runs_in_the_image_world(monkeypatch): + """The blessed recipe: bash.run(cmd, env=image_env()) drops the bundle's + injected PATH and restores the image's stripped vars.""" + from agentix.bash import image_env + from agentix.runtime.shared.env import AGENTIX_ADDED_PATH real_path = os.environ.get("PATH", "/usr/bin") monkeypatch.setenv("PATH", os.pathsep.join(["/agentix-injected/bin", real_path])) monkeypatch.setenv(AGENTIX_ADDED_PATH, "/agentix-injected/bin") monkeypatch.setenv("AGENTIX_SAVED_PYTHONPATH", "/testbed/src") - # restore is live-wins: an ambient PYTHONPATH (conda, IDE runners) would - # legitimately shadow the snapshot and flake this test — clear it. monkeypatch.delenv("PYTHONPATH", raising=False) - polluted = await bash.run(command="printf %s \"$PATH\"") + polluted = await bash.run(command='printf %s "$PATH"') assert polluted.stdout.startswith("/agentix-injected/bin") - clean = await bash.run(command="printf %s \"$PATH:$PYTHONPATH\"", clean_env=True) + clean = await bash.run(command='printf %s "$PATH:$PYTHONPATH"', env=image_env()) assert "/agentix-injected/bin" not in clean.stdout assert clean.stdout.endswith("/testbed/src") -def test_shell_executable_prefers_image_bash_in_clean_mode(tmp_path): - """A clean-env command's restored vars (LD_PRELOAD, ...) target the - image's own bash; the bundled Nix bash is only the last resort.""" - fakebin = tmp_path / "bin" - fakebin.mkdir() - image_bash = fakebin / "bash" - image_bash.write_text("#!/bin/sh\n") - image_bash.chmod(0o755) - - resolved = bash._shell_executable(None, {"PATH": str(fakebin)}, clean=True) - assert resolved == str(image_bash) - - # without clean mode the bundled-bash preference is unchanged (falls back - # to PATH lookup here because /nix/runtime/bin/bash does not exist) - assert bash._shell_executable(None, {"PATH": str(fakebin)}) == str(image_bash) def _set_files_root(monkeypatch, root): # UPLOAD_ROOT is computed from env at import time; patch the module # attribute directly — reloading the module would re-create its classes @@ -303,3 +327,33 @@ async def test_files_link_resolving_exactly_to_root_then_suffix(tmp_path, monkey _set_files_root(monkeypatch, root) assert await files.download("sub/up/target.txt") == b"hit" + + +def test_image_env_bashrc_follows_the_env_home(tmp_path, monkeypatch): + """BASH_ENV must point at the RESULT env's ~/.bashrc — including when the + image's HOME was stripped+restored or overridden via extra — not at the + worker process's ambient HOME.""" + from agentix.bash import image_env + + (tmp_path / ".bashrc").write_text("export FROM_IMAGE_RC=1\n") + env = image_env({"HOME": str(tmp_path)}) + assert env["BASH_ENV"] == str(tmp_path / ".bashrc") + + explicit = image_env({"HOME": str(tmp_path), "BASH_ENV": "/custom/rc"}) + assert explicit["BASH_ENV"] == "/custom/rc" # caller's value is preserved + + +def test_shell_executable_follows_the_env_path(tmp_path): + """The shell resolves from the env's PATH like everything else the + command runs: an image_env() PATH selects the image's bash (its restored + LD_PRELOAD etc. target that binary); no bash on PATH falls back.""" + fakebin = tmp_path / "bin" + fakebin.mkdir() + image_bash = fakebin / "bash" + image_bash.write_text("#!/bin/sh\n") + image_bash.chmod(0o755) + + assert bash._shell_executable(None, {"PATH": str(fakebin)}) == str(image_bash) + # no bash anywhere on the supplied PATH -> /bin/bash fallback + # (BUNDLE_RUNTIME_BASH does not exist outside a bundle) + assert bash._shell_executable(None, {"PATH": str(tmp_path)}) == "/bin/bash" diff --git a/tests/runtime/test_env.py b/tests/runtime/test_env.py index 991cf36..74e777c 100644 --- a/tests/runtime/test_env.py +++ b/tests/runtime/test_env.py @@ -2,7 +2,7 @@ import os -from agentix.bash import _clean_env +from agentix.bash import _build_env, image_env from agentix.runtime.shared.env import ( AGENTIX_ADDED_LD_LIBRARY_PATH, @@ -85,14 +85,16 @@ def test_get_env_without_agentix_applies_extra_last() -> None: assert env[AGENTIX_ADDED_PATH] == "/caller/value" -def test_bash_clean_env_inherits_agentix_runtime_env(monkeypatch) -> None: - monkeypatch.setenv("PATH", os.pathsep.join(["/nix/runtime/bin", "/task/bin"])) - monkeypatch.setenv(AGENTIX_ADDED_PATH, "/nix/runtime/bin") +def test_build_env_none_inherits_and_dict_replaces(monkeypatch) -> None: + """bash env follows subprocess.run: None inherits the worker env, a dict + replaces it wholesale.""" + monkeypatch.setenv("AGENTIX_TEST_MARKER", "present") - env = _clean_env(None) + inherited = _build_env(None) + assert inherited.get("AGENTIX_TEST_MARKER") == "present" - assert env["PATH"].split(os.pathsep) == ["/nix/runtime/bin", "/task/bin"] - assert env[AGENTIX_ADDED_PATH] == "/nix/runtime/bin" + replaced = _build_env({"ONLY": "1"}) + assert replaced == {"ONLY": "1"} def test_get_env_without_agentix_restores_saved_vars() -> None: @@ -135,12 +137,13 @@ def test_get_env_without_agentix_extra_wins_over_restored() -> None: assert env["PYTHONPATH"] == "/caller" -def test_bash_clean_env_opt_in_builds_the_image_environment(monkeypatch) -> None: +def test_image_env_builds_the_task_image_environment(monkeypatch) -> None: monkeypatch.setenv("PATH", os.pathsep.join(["/nix/runtime/bin", "/task/bin"])) monkeypatch.setenv(AGENTIX_ADDED_PATH, "/nix/runtime/bin") monkeypatch.setenv("AGENTIX_SAVED_PYTHONPATH", "/testbed/src") + monkeypatch.delenv("PYTHONPATH", raising=False) - env = _clean_env({"EXTRA": "1"}, clean=True) + env = image_env({"EXTRA": "1"}) assert env["PATH"] == "/task/bin" assert env["PYTHONPATH"] == "/testbed/src"