diff --git a/fern/versions/latest/pages/agent-server/add-or-update-a-harness.mdx b/fern/versions/latest/pages/agent-server/add-or-update-a-harness.mdx index 4e5c807668d..34f226c26fb 100644 --- a/fern/versions/latest/pages/agent-server/add-or-update-a-harness.mdx +++ b/fern/versions/latest/pages/agent-server/add-or-update-a-harness.mdx @@ -404,7 +404,7 @@ Keep harness-specific response parsing and model and tool behavior in the adapte **1. Define the session state.** Subclass `AgentSessionState` and compose a `SandboxSession`. Implement two hooks: -- `stage_activation` uploads the activation input and returns `SandboxCommand(argv=..., python=...)`. `python` is the sandbox interpreter that runs the supervisor. Install the runtime earlier, during seed. +- `stage_activation` uploads the activation input and returns `SandboxCommand(argv=..., python=...)`. `python` is the sandbox interpreter that runs the supervisor. Install the runtime earlier, during seed. Set `stdlib_runner=True` when the runner needs only the standard library and `argv` starts with `python`: the runner then runs as `bin/gym-runner`, like the supervisor, so a `pkill -f python3` in the task cannot stop it. Upload such a runner without a `.py` suffix, as Codex and Pi do. - `collect` copies artifacts to the Agent Server after harness termination is confirmed, including partial output from interrupted runs. It may run a bounded snapshot command, but it must not restart the harness. Bound the size of what it downloads: trim large logs inside the sandbox first and keep the tail with a truncation marker. For example, the older `hermes_sandboxed_agent`, which calls the process supervisor directly, keeps the last 1 MiB of its runner log; apply the same bound in `collect`. ```python @@ -463,7 +463,7 @@ async with self.sem: # asyncio.Semaphore(self.config.concurrency) ) ``` -`execute()` uploads `process_supervisor.py` into `session_dir` after `stage_activation` returns, so adapters do not upload it themselves. It then launches the harness under the supervisor, confirms harness termination, collects artifacts, and releases the provider exec. Captured artifacts remain in `session.artifacts` even when execution raises. If the supervisor writes a final negative receipt, `execute()` raises `HarnessTerminationError` and does not capture artifacts. A nonzero harness exit code does not raise by itself, because the output can still be useful. Convert artifacts into a response and observations in the adapter. +`execute()` uploads `process_supervisor.py` into `session_dir`, as `process_supervisor`, after `stage_activation` returns, so adapters do not upload it themselves. It then launches the harness under the supervisor, confirms harness termination, collects artifacts, and releases the provider exec. Captured artifacts remain in `session.artifacts` even when execution raises. If the supervisor writes a final negative receipt, `execute()` raises `HarnessTerminationError` and does not capture artifacts. A nonzero harness exit code does not raise by itself, because the output can still be useful. Convert artifacts into a response and observations in the adapter. **4. Close the session.** In `_close_agent_session_state()`, call `session.close()` first, then cancel the activation task if it is still running. Close joins any finalization in progress, never captures artifacts twice, and is safe to retry while termination evidence is missing. After a final negative receipt, close skips capture, releases a borrowed connection, and leaves teardown to the Resources Server; an owned sandbox is stopped through the provider. Build observations from `session.artifacts` if activation was interrupted before parsing them. @@ -496,7 +496,9 @@ All control files live under `session_dir`, and their names are defined in `supe | File | Writer and purpose | | --- | --- | -| `process_supervisor.py` | `SandboxSession` uploads it after staging input and before launch. | +| `process_supervisor` | `SandboxSession` uploads `process_supervisor.py` under this name after staging input and before launch. | +| `bin/gym-supervisor` | The launch shell links it to the binary behind `SandboxCommand.python` and runs the supervisor through it, so the supervisor's process name doesn't mention python. If the link can't be made, the supervisor runs under `python`. | +| `bin/gym-runner` | The same link for a runner that sets `stdlib_runner`. | | `launch.claim` | The launch or stop shell creates it atomically, either to claim execution or to fence a delayed launch. | | `supervisor.pid` | The launch shell records the PID that becomes the supervisor when the shell executes it. | | `stop.request` | The stop shell writes this cancellation marker before signaling the supervisor. | diff --git a/nemo_gym/agent_utils/sandbox_session.py b/nemo_gym/agent_utils/sandbox_session.py index 34248652296..861044c2db0 100644 --- a/nemo_gym/agent_utils/sandbox_session.py +++ b/nemo_gym/agent_utils/sandbox_session.py @@ -17,6 +17,7 @@ STOP_REQUEST_FILE, SUPERVISOR_FILE, HarnessTerminationError, + check_stdlib_runner, remove_session_directory, stop_and_confirm_cleanup, supervised_launch_command, @@ -28,6 +29,9 @@ LOG = logging.getLogger(__name__) +# Where adapters upload a standard-library runner, relative to session_dir. Without a +# .py suffix, a `pkill -f '\.py'` in the task cannot match the runner's command line. +RUNNER_FILE = "sandbox_runner" def _reuse_or_start[T]( @@ -40,10 +44,20 @@ def _reuse_or_start[T]( @dataclass(frozen=True, kw_only=True) class SandboxCommand: - """Harness argv and the installed interpreter used to launch its supervisor.""" + """Harness argv and the installed interpreter used to launch its supervisor. + + ``stdlib_runner`` declares that the runner needs nothing beyond the standard library. + Its argv, which must start with ``python``, then runs under a neutral process name, + as the supervisor always does, so that ``pkill python3`` in the task cannot stop it. + The renamed interpreter is the base interpreter, outside any virtual environment. + """ argv: list[str] python: str + stdlib_runner: bool = False + + def __post_init__(self) -> None: + check_stdlib_runner(self.argv, python=self.python, stdlib_runner=self.stdlib_runner) @dataclass(kw_only=True) @@ -169,6 +183,7 @@ async def execute( timeout=timeout, cleanup_timeout=cleanup_timeout, python=command.python, + stdlib_runner=command.stdlib_runner, ) self.launch_started = True self._exec_task = asyncio.create_task( diff --git a/nemo_gym/agent_utils/supervisor_client.py b/nemo_gym/agent_utils/supervisor_client.py index af5bbdb2d82..641c092ccc4 100644 --- a/nemo_gym/agent_utils/supervisor_client.py +++ b/nemo_gym/agent_utils/supervisor_client.py @@ -10,13 +10,25 @@ ===================== ==================== =========================================== Filename Writer When ===================== ==================== =========================================== -process_supervisor.py Session Uploaded before launch +process_supervisor Session Uploaded before launch +bin/gym-supervisor Launch shell Interpreter link for the supervisor +bin/gym-runner Launch shell Interpreter link for a standard-library runner launch.claim Launch or stop shell Atomically claims launch or fences it supervisor.pid Launch shell After claiming launch, before exec stop.request Stop shell Before signalling the supervisor cleanup.json Supervisor or stop After cleanup, or when stop fences launch output.log Launch shell Captures supervisor and harness diagnostics ===================== ==================== =========================================== + +Agents in the task sandbox often clean up with ``pkill -f python3``, ``pkill python3`` or +``killall python3``. In a reference harness whose agent CLI is a native binary these +commands are harmless; here they would stop the supervisor, which then ends the +harness. So the supervisor, and a standard-library runner that opts in, run under +interpreter links with neutral names, from files without a ``.py`` suffix. Their process +names never mention python, and neither do their command lines when the runner opts in; a +harness argv that keeps its own python (Hermes, Stirrup) still appears in the supervisor's +command line. Gym's own stop path signals the recorded supervisor PID and does not depend +on these names. """ import json @@ -31,7 +43,9 @@ from nemo_gym.sandbox.utils import read_text -SUPERVISOR_FILE = "process_supervisor.py" +SUPERVISOR_FILE = "process_supervisor" +SUPERVISOR_INTERPRETER = "bin/gym-supervisor" +RUNNER_INTERPRETER = "bin/gym-runner" LAUNCH_CLAIM_FILE = "launch.claim" SUPERVISOR_PID_FILE = "supervisor.pid" STOP_REQUEST_FILE = "stop.request" @@ -39,6 +53,8 @@ OUTPUT_LOG_FILE = "output.log" _CLEANUP_RECEIPT = TypeAdapter(CleanupReceipt) +# The interpreter binary itself, past virtual-environment links and launcher shims. +_RESOLVE_INTERPRETER = 'import os; print(os.readlink("/proc/self/exe"))' LOG = logging.getLogger(__name__) @@ -91,6 +107,14 @@ def supervision_timeouts(*, timeout: float, close_timeout: float) -> tuple[float return cleanup_timeout, exec_timeout(timeout=timeout, cleanup_timeout=cleanup_timeout) +def check_stdlib_runner(command: list[str], *, python: str, stdlib_runner: bool) -> None: + """Reject ``stdlib_runner`` for a harness argv that the runner link cannot replace.""" + if stdlib_runner and command[:1] != [python]: + raise ValueError( + f"stdlib_runner requires a harness argv that starts with the interpreter {python!r}; got {command[:1]!r}" + ) + + def supervised_launch_command( *, session_dir: str, @@ -98,6 +122,7 @@ def supervised_launch_command( timeout: float, cleanup_timeout: float, python: str, + stdlib_runner: bool = False, ) -> str: """Fence delayed launches and run a harness command under the shared supervisor. @@ -105,18 +130,37 @@ def supervised_launch_command( can load the supervisor before claiming a launch: a failed bootstrap cannot write a cleanup receipt. Supervisor and harness diagnostics share output.log. The supervisor stops the harness group; the sandbox owner stops escaped services. + + The supervisor runs through ``bin/gym-supervisor``, a link to the binary behind + ``python``, so that python-matching kills in the task skip it. With ``stdlib_runner``, + the harness command, which must start with ``python``, runs through ``bin/gym-runner`` + the same way. The links resolve a virtual environment to its base interpreter, so only + standard-library runners may opt in. When the link cannot be made or cannot load + the supervisor, both processes keep ``python``. """ + check_stdlib_runner(command, python=python, stdlib_runner=stdlib_runner) supervisor = quote(f"{session_dir}/{SUPERVISOR_FILE}") claim_path = quote(f"{session_dir}/{LAUNCH_CLAIM_FILE}") + link = quote(f"{session_dir}/{SUPERVISOR_INTERPRETER}") + harness = join(command) + runner = "" + if stdlib_runner: + runner_link = quote(f"{session_dir}/{RUNNER_INTERPRETER}") + runner = f'run={quote(python)}; [ "$py" = {link} ] && ln -sfn "$real" {runner_link} && run={runner_link}; ' + harness = f'"$run" {join(command[1:])}' return ( f"[ -d {quote(session_dir)} ] && [ ! -L {claim_path} ] || exit 0; " - f"{quote(python)} -I {supervisor} --help >/dev/null || exit $?; " - f"trap '' TERM; ln -s launch {claim_path} 2>/dev/null || exit 0; " + f"py={quote(python)}; " + f"real=$({quote(python)} -I -c {quote(_RESOLVE_INTERPRETER)} 2>/dev/null) && " + f'{{ [ -d {quote(f"{session_dir}/bin")} ] || mkdir {quote(f"{session_dir}/bin")}; }} && ln -sfn "$real" {link} && ' + f"{link} -I {supervisor} --help >/dev/null 2>&1 && py={link}; " + f'[ "$py" = {link} ] || {quote(python)} -I {supervisor} --help >/dev/null || exit $?; ' + f"{runner}trap '' TERM; ln -s launch {claim_path} 2>/dev/null || exit 0; " f"echo $$ > {quote(f'{session_dir}/{SUPERVISOR_PID_FILE}')} && " - f"exec {quote(python)} -I {supervisor} " + f'exec "$py" -I {supervisor} ' f"--timeout {timeout} --cleanup-timeout {cleanup_timeout} " f"--stop-file {quote(f'{session_dir}/{STOP_REQUEST_FILE}')} " - f"--receipt {quote(f'{session_dir}/{CLEANUP_RECEIPT_FILE}')} -- {join(command)} " + f"--receipt {quote(f'{session_dir}/{CLEANUP_RECEIPT_FILE}')} -- {harness} " f">{quote(f'{session_dir}/{OUTPUT_LOG_FILE}')} 2>&1" ) diff --git a/responses_api_agents/codex_agent/README.md b/responses_api_agents/codex_agent/README.md index eb56e7624d7..2774a600c26 100644 --- a/responses_api_agents/codex_agent/README.md +++ b/responses_api_agents/codex_agent/README.md @@ -157,6 +157,9 @@ blocks verification. Missing evidence stays retryable; a final negative receipt the borrowed connection without making the failed activation gradable. The borrowing agent never destroys the sandbox. Cleanup coordinates lifecycle; it is not a security boundary against hostile task code. +The supervisor and the runner above Codex run as `gym-supervisor` and `gym-runner` (links to the +sandbox's `python3`) from files without a `.py` suffix, so a task agent's `pkill -f python3`, +`pkill python3` or `killall python3` does not end Codex, as it would not under Harbor. Responses and close observations preserve text, reasoning, tool events, partial output, CLI aggregate usage, and sandbox hostname/PID/version. Wall-time and the pinned CLI's explicit diff --git a/responses_api_agents/codex_agent/app.py b/responses_api_agents/codex_agent/app.py index e14a8686bd4..6ef63a4c9ff 100644 --- a/responses_api_agents/codex_agent/app.py +++ b/responses_api_agents/codex_agent/app.py @@ -35,7 +35,7 @@ from pydantic import ConfigDict, Field, PrivateAttr, model_validator from nemo_gym.agent_outcome import AgentOutcome -from nemo_gym.agent_utils.sandbox_session import SandboxSession +from nemo_gym.agent_utils.sandbox_session import RUNNER_FILE, SandboxSession from nemo_gym.agent_utils.task_timeout import AGENT_TIMEOUT_METADATA_KEY, resolve_agent_timeout from nemo_gym.base_resources_server import NEMO_GYM_MCP_METADATA_KEY, BaseRunRequest, BaseVerifyResponse from nemo_gym.base_responses_api_agent import ( @@ -729,8 +729,9 @@ async def _seed_agent_session_state(self, body: AgentSeedSessionRequest) -> Code f"Codex installer exited {installed.return_code}, command={install_command}, " f"error_type={installed.error_type}; stdout={installed.stdout}; stderr={installed.stderr}" ) - for filename in ("sandbox_runner.py", "sandbox_transport.py"): - await sandbox.upload(Path(__file__).with_name(filename), f"{directory}/{filename}") + await sandbox.upload(Path(__file__).with_name("sandbox_runner.py"), f"{directory}/{RUNNER_FILE}") + # The runner loads the transport from beside itself under this name. + await sandbox.upload(Path(__file__).with_name("sandbox_transport.py"), f"{directory}/sandbox_transport.py") except BaseException as error: try: await state.close(self.config.session_close_timeout_seconds) diff --git a/responses_api_agents/codex_agent/sandbox.py b/responses_api_agents/codex_agent/sandbox.py index d667fe374b6..5b009ea1082 100644 --- a/responses_api_agents/codex_agent/sandbox.py +++ b/responses_api_agents/codex_agent/sandbox.py @@ -10,7 +10,7 @@ from pydantic import BaseModel, ConfigDict, JsonValue, ValidationError -from nemo_gym.agent_utils.sandbox_session import SandboxCommand, SandboxSession +from nemo_gym.agent_utils.sandbox_session import RUNNER_FILE, SandboxCommand, SandboxSession from nemo_gym.base_responses_api_agent import AgentSessionState from nemo_gym.openai_utils import NeMoGymResponse, NeMoGymResponseCreateParamsNonStreaming from nemo_gym.rollout_observability import AgentObservationBundle @@ -96,9 +96,10 @@ async def stage_activation(self, payload: dict[str, JsonValue]) -> SandboxComman argv=[ "python3", "-I", - f"{self.session.session_dir}/sandbox_runner.py", + f"{self.session.session_dir}/{RUNNER_FILE}", f"{self.session.session_dir}/input.json", ], + stdlib_runner=True, ) async def collect_artifacts(self) -> str: @@ -119,7 +120,7 @@ async def collect_artifacts(self) -> str: self.runtime_info = parse_runtime_info(runtime_payload) try: snapshot = await self.session.sandbox.exec( - f"python3 -I {shlex.quote(self.session.session_dir + '/sandbox_runner.py')} " + f"python3 -I {shlex.quote(f'{self.session.session_dir}/{RUNNER_FILE}')} " f"--snapshot {shlex.quote(self.session.session_dir)}", timeout_s=10, ) diff --git a/responses_api_agents/codex_agent/tests/test_exec_transport.py b/responses_api_agents/codex_agent/tests/test_exec_transport.py index 5e91ba0273c..8f6ee726e5c 100644 --- a/responses_api_agents/codex_agent/tests/test_exec_transport.py +++ b/responses_api_agents/codex_agent/tests/test_exec_transport.py @@ -40,7 +40,7 @@ async def disconnect(self): self.disconnected = True async def exec(self, command, *, cwd=None, timeout_s=30): - launch = "--receipt" in command and "process_supervisor.py" in command + launch = "--receipt" in command and "process_supervisor" in command if launch: self.deadline = timeout_s self.delayed_command = command @@ -77,7 +77,7 @@ def make_session(tmp_path): ), runtime=str(tmp_path / "runtime"), ) - shutil.copyfile(sandbox_runner.__file__, directory / "sandbox_runner.py") + shutil.copyfile(sandbox_runner.__file__, directory / "sandbox_runner") return state, provider, workdir diff --git a/responses_api_agents/codex_agent/tests/test_sandbox_sessions.py b/responses_api_agents/codex_agent/tests/test_sandbox_sessions.py index 0e1fa0f61fd..b1bd69d5208 100644 --- a/responses_api_agents/codex_agent/tests/test_sandbox_sessions.py +++ b/responses_api_agents/codex_agent/tests/test_sandbox_sessions.py @@ -124,7 +124,7 @@ def publish(self): self.files[f"{self.directory}/events.jsonl"] = self.events async def execute(self, command, **kwargs): - if "exec python3 -I " in command and "process_supervisor.py" in command: + if "--receipt" in command and "process_supervisor" in command: return await self.launch(command=command, **kwargs) if "touch " in command and "stop.request" in command: self.signals.append("SIGTERM") @@ -139,7 +139,7 @@ async def create(self, **kwargs): payload = json.loads(self.files[payload_path]) assert payload["cwd"] == getattr(self, "expected_workdir", "/app") assert kwargs["cwd"] == getattr(self, "expected_workdir", "/app") - assert "sandbox_runner.py" in kwargs["command"] + assert "/sandbox_runner " in kwargs["command"] and "/bin/gym-runner" in kwargs["command"] self.directory = payload["directory"] self.started.set() if not self.blocked: @@ -192,11 +192,12 @@ def test_http_agent_session_flow_runs_codex_in_borrowed_sandbox(setup): assert created.status_code == 200, created.text session_id = created.json()["agent_session_id"] directory = agent._session_records[session_id].state.session.session_dir - assert f"{directory}/process_supervisor.py" not in sandbox.files + assert f"{directory}/process_supervisor" not in sandbox.files installer = f"{directory}/install_codex_runtime.sh" assert installer in sandbox.files - for filename in ("sandbox_runner.py", "sandbox_transport.py"): - assert sandbox.files[f"{directory}/{filename}"] == (Path(__file__).parents[1] / filename).read_text() + staged = {"sandbox_runner": "sandbox_runner.py", "sandbox_transport.py": "sandbox_transport.py"} + for name, source in staged.items(): + assert sandbox.files[f"{directory}/{name}"] == (Path(__file__).parents[1] / source).read_text() assert agent.config.resources_server is None assert not sandbox.launch.called assert not any(path.startswith("/app/") for path in sandbox.files) @@ -205,7 +206,7 @@ def test_http_agent_session_flow_runs_codex_in_borrowed_sandbox(setup): assert install_call.kwargs["timeout_s"] == agent.config.sandbox_install_timeout_seconds result = client.post("/ng-rollout/codex-smoke-a2/v1/responses", json={"input": "Fix the code"}) assert result.status_code == 200, result.text - assert f"{directory}/process_supervisor.py" in sandbox.files + assert f"{directory}/process_supervisor" in sandbox.files body = result.json() assert body["status"] == "completed" assert [item["type"] for item in body["output"]] == [ @@ -1735,7 +1736,7 @@ async def local_session(setup, tmp_path): state.session.session_dir = str(directory) from responses_api_agents.codex_agent import sandbox_runner - shutil.copyfile(sandbox_runner.__file__, directory / "sandbox_runner.py") + shutil.copyfile(sandbox_runner.__file__, directory / "sandbox_runner") sandbox.upload = AsyncMock(side_effect=shutil.copyfile) sandbox.download = AsyncMock(side_effect=shutil.copyfile) @@ -1774,7 +1775,7 @@ async def test_close_fences_delayed_launch_before_and_after_removing_session(loc commands = [] async def queued_exec(command, **kwargs): - if "--receipt" in command and "process_supervisor.py" in command: + if "--receipt" in command and "process_supervisor" in command: commands.append(command) waiting.set() if failure == "cancel": diff --git a/responses_api_agents/hermes_agent/tests/test_app.py b/responses_api_agents/hermes_agent/tests/test_app.py index 7ac19c3965e..ee64d65cfec 100644 --- a/responses_api_agents/hermes_agent/tests/test_app.py +++ b/responses_api_agents/hermes_agent/tests/test_app.py @@ -492,7 +492,7 @@ async def download(self, remote_path, local_path) -> None: response = await hermes.responses(request, NeMoGymResponseCreateParamsNonStreaming(input="fix bug")) runner_input = json.loads(sandbox.uploaded["/session/input.json"]) - assert "/session/process_supervisor.py" in sandbox.uploaded + assert "/session/process_supervisor" in sandbox.uploaded assert runner_input["model_base_url"] == f"http://model-server:1/ng-rollout/{seed.episode_id.capture_key}/v1" assert runner_input["user_message"] == "fix bug" assert response.metadata["harness_execution"] == "sandbox" diff --git a/responses_api_agents/hermes_agent/tests/test_sandbox.py b/responses_api_agents/hermes_agent/tests/test_sandbox.py index e8b253c1ede..df0e42d4c62 100644 --- a/responses_api_agents/hermes_agent/tests/test_sandbox.py +++ b/responses_api_agents/hermes_agent/tests/test_sandbox.py @@ -743,7 +743,7 @@ async def test_stop_during_interpreter_startup_closes_without_starting_a_worker( "raise SystemExit(runner['main']())\n" ) state.session.sandbox.upload.side_effect = lambda source, destination: shutil.copyfile( - wrapper if Path(source).name == SUPERVISOR_FILE else source, destination + wrapper if Path(destination).name == SUPERVISOR_FILE else source, destination ) if stop_timing == "before-shell": (directory / STOP_REQUEST_FILE).touch() @@ -931,7 +931,7 @@ async def test_session_prompt_and_limits_reach_runner(agent, state, overrides, t await agent._run_sandbox_episode(body=body, agent_session_id="session", state=state) launch_command = state.session.sandbox.exec.await_args_list[0].args[0] launch_args = shlex.split(launch_command) - assert any(arg.endswith("/process_supervisor.py") for arg in launch_args) + assert any(arg.endswith("/process_supervisor") for arg in launch_args) assert float(launch_args[launch_args.index("--timeout") + 1]) == agent.config.sandbox_runner_timeout_seconds cleanup_timeout = float(launch_args[launch_args.index("--cleanup-timeout") + 1]) assert cleanup_timeout * process_supervisor.CLEANUP_PHASE_COUNT == pytest.approx( diff --git a/responses_api_agents/kilocode_agent/tests/test_exec_transport.py b/responses_api_agents/kilocode_agent/tests/test_exec_transport.py index 937e5649d52..76af8acbe83 100644 --- a/responses_api_agents/kilocode_agent/tests/test_exec_transport.py +++ b/responses_api_agents/kilocode_agent/tests/test_exec_transport.py @@ -40,7 +40,7 @@ async def disconnect(self): self.disconnected = True async def exec(self, command, *, cwd=None, timeout_s=30): - launch = "--receipt" in command and "process_supervisor.py" in command + launch = "--receipt" in command and "process_supervisor" in command if launch: self.deadline = timeout_s self.delayed_command = command @@ -75,7 +75,7 @@ def make_session(tmp_path): session=SandboxSession(sandbox=provider, session_dir=str(directory), workdir=str(workdir), harness="KiloCode"), ) shutil.copyfile(sandbox_runner.__file__, directory / "sandbox_runner.py") - assert not (directory / "process_supervisor.py").exists() + assert not (directory / "process_supervisor").exists() return state, provider, workdir diff --git a/responses_api_agents/kilocode_agent/tests/test_lifecycle_contract.py b/responses_api_agents/kilocode_agent/tests/test_lifecycle_contract.py index 0b1fb1edd76..1a797b7720f 100644 --- a/responses_api_agents/kilocode_agent/tests/test_lifecycle_contract.py +++ b/responses_api_agents/kilocode_agent/tests/test_lifecycle_contract.py @@ -28,7 +28,7 @@ def __init__(self): self.disconnect = AsyncMock() async def run_exec(self, command, **kwargs): - if "--receipt" in command and "process_supervisor.py" in command: + if "--receipt" in command and "process_supervisor" in command: self.events.append("launch") self.files[f"{DIRECTORY}/stdout.jsonl"] = '{"type":"text","part":{"text":"done"}}\n' self.files[f"{DIRECTORY}/stderr.log"] = "" diff --git a/responses_api_agents/nooa_agent/tests/test_checkpoint_recovery.py b/responses_api_agents/nooa_agent/tests/test_checkpoint_recovery.py index d96a2eea58f..1d906b22687 100644 --- a/responses_api_agents/nooa_agent/tests/test_checkpoint_recovery.py +++ b/responses_api_agents/nooa_agent/tests/test_checkpoint_recovery.py @@ -53,7 +53,7 @@ async def download(self, source: str, destination: Path) -> None: async def exec( self, command: str, *, cwd: str, timeout_s: float | None, preserve_background_services: bool = False ) -> SandboxExecResult: - if "process_supervisor.py" in command: + if "process_supervisor" in command: # Run the actual entrypoint from this checkout. The installed sandbox's # isolated interpreter and Linux descendant reaper are not under test. with (self.directory / "output.log").open("wb") as stdout: diff --git a/responses_api_agents/nooa_agent/tests/test_sandbox_runner.py b/responses_api_agents/nooa_agent/tests/test_sandbox_runner.py index 7b5d0d69802..2f5cf488982 100644 --- a/responses_api_agents/nooa_agent/tests/test_sandbox_runner.py +++ b/responses_api_agents/nooa_agent/tests/test_sandbox_runner.py @@ -88,8 +88,8 @@ async def test_launch_quotes_paths_runs_in_task_and_preserves_cookies(limit: int assert "'/opt/nooa env/bin/python'" in command assert "-I -m responses_api_agents.nooa_agent.sandbox_entrypoint" in command assert "sandbox_supervisor" not in command - assert "/process_supervisor.py" in command - assert r.directory + "/process_supervisor.py" in s.files + assert "/process_supervisor " in command + assert r.directory + "/process_supervisor" in s.files launch_command = await r._stage_activation(request) assert launch_command.argv[-2:] == [r.directory + "/input.json", r.directory + "/result.json"] assert len(launch_command.argv) == 6 @@ -393,7 +393,7 @@ async def upload(source: Path, destination: str) -> None: with pytest.raises(asyncio.CancelledError): await execution assert not r.session.launch_started - assert all("process_supervisor.py" not in call.args[0] for call in s.exec.await_args_list) + assert all("process_supervisor" not in call.args[0] for call in s.exec.await_args_list) s.disconnect.assert_awaited_once() @@ -551,7 +551,7 @@ async def download(source: str, destination: Path) -> None: await original_download(source, destination) async def execute(command: str, **kwargs) -> SandboxExecResult: - if "process_supervisor.py" in command: + if "process_supervisor" in command: raise original_error return SandboxExecResult(stdout="", stderr="", return_code=0) @@ -697,7 +697,7 @@ async def test_cancelling_active_worker_captures_checkpoint_before_release() -> execution_stopped = asyncio.Event() async def execute(command: str, **kwargs) -> SandboxExecResult: - if "process_supervisor.py" in command: + if "process_supervisor" in command: active.set() try: await asyncio.Event().wait() @@ -896,7 +896,7 @@ async def test_cancel_without_any_artifact_preserves_bare_cancellation() -> None active = asyncio.Event() async def execute(command: str, **kwargs) -> SandboxExecResult: - if "process_supervisor.py" in command: + if "process_supervisor" in command: active.set() await asyncio.Event().wait() elif command.startswith("[ -f "): @@ -984,7 +984,7 @@ async def download_artifact(source: str, destination: Path) -> None: await download(source, destination) async def execute(command: str, **kwargs) -> SandboxExecResult: - if "process_supervisor.py" in command: + if "process_supervisor" in command: raise provider_error return SandboxExecResult("", "", 0) diff --git a/responses_api_agents/openclaw_agent/README.md b/responses_api_agents/openclaw_agent/README.md index 048b69b06e9..27fc20f9a1b 100644 --- a/responses_api_agents/openclaw_agent/README.md +++ b/responses_api_agents/openclaw_agent/README.md @@ -245,6 +245,9 @@ Natural CLI exit cannot distinguish unfinished tools in other sessions from serv those processes may still change task files. The stop-time cutoff also has adoption/PID races and cannot classify later service forks. It is not a security boundary. The sandbox launcher sets `OPENCLAW_NO_RESPAWN=1` to keep the CLI in the supervised group. +The supervisor and the runner above OpenClaw run as `gym-supervisor` and `gym-runner` (links to the +sandbox's `python3`) from files without a `.py` suffix, so a task agent's `pkill -f python3`, +`pkill python3` or `killall python3` does not end OpenClaw. Services must use task-owned files rather than the temporary agent session directory. Close is serialized and checks the receipt, process handle, adapter-owned file removal, and disconnect before succeeding. Unknown launch outcomes and missing or negative termination evidence block verification. Missing evidence remains retryable; diff --git a/responses_api_agents/openclaw_agent/app.py b/responses_api_agents/openclaw_agent/app.py index 1f399624af0..3a63a10b04c 100644 --- a/responses_api_agents/openclaw_agent/app.py +++ b/responses_api_agents/openclaw_agent/app.py @@ -35,7 +35,7 @@ from fastapi import HTTPException, Request from pydantic import ConfigDict, Field, PrivateAttr -from nemo_gym.agent_utils.sandbox_session import SandboxSession +from nemo_gym.agent_utils.sandbox_session import RUNNER_FILE, SandboxSession from nemo_gym.agent_utils.task_timeout import AGENT_TIMEOUT_METADATA_KEY, resolve_agent_timeout from nemo_gym.base_resources_server import BaseRunRequest, BaseVerifyResponse from nemo_gym.base_responses_api_agent import ( @@ -578,7 +578,7 @@ async def _seed_agent_session_state(self, body: AgentSeedSessionRequest) -> Open f"bash {directory}/{installer} {runtime} {self.config.openclaw_version}\n" f"stderr={(installed.stderr or '')[-16000:]}; stdout={(installed.stdout or '')[-16000:]}" ) - await sandbox.upload(Path(__file__).with_name("sandbox_runner.py"), f"{directory}/sandbox_runner.py") + await sandbox.upload(Path(__file__).with_name("sandbox_runner.py"), f"{directory}/{RUNNER_FILE}") except BaseException as error: try: if prepared_directory or owns_sandbox: diff --git a/responses_api_agents/openclaw_agent/sandbox.py b/responses_api_agents/openclaw_agent/sandbox.py index 378cdc1d4fb..f683a0e867d 100644 --- a/responses_api_agents/openclaw_agent/sandbox.py +++ b/responses_api_agents/openclaw_agent/sandbox.py @@ -9,7 +9,7 @@ from pydantic import BaseModel, ConfigDict, JsonValue, ValidationError -from nemo_gym.agent_utils.sandbox_session import SandboxCommand, SandboxSession +from nemo_gym.agent_utils.sandbox_session import RUNNER_FILE, SandboxCommand, SandboxSession from nemo_gym.base_responses_api_agent import AgentSessionState from nemo_gym.openai_utils import NeMoGymResponse, NeMoGymResponseCreateParamsNonStreaming from nemo_gym.rollout_observability import AgentObservationBundle @@ -107,9 +107,10 @@ async def stage_activation(self, payload: dict[str, JsonValue]) -> SandboxComman argv=[ "python3", "-I", - f"{self.session.session_dir}/sandbox_runner.py", + f"{self.session.session_dir}/{RUNNER_FILE}", f"{self.session.session_dir}/input.json", ], + stdlib_runner=True, ) async def collect_artifacts(self) -> dict[str, str]: diff --git a/responses_api_agents/openclaw_agent/tests/test_exec_transport.py b/responses_api_agents/openclaw_agent/tests/test_exec_transport.py index 0ce32fff188..4cf350e9925 100644 --- a/responses_api_agents/openclaw_agent/tests/test_exec_transport.py +++ b/responses_api_agents/openclaw_agent/tests/test_exec_transport.py @@ -40,7 +40,7 @@ async def disconnect(self): self.disconnected = True async def exec(self, command, *, cwd=None, timeout_s=30): - launch = "--receipt" in command and "process_supervisor.py" in command + launch = "--receipt" in command and "process_supervisor" in command if launch: self.deadline = timeout_s self.delayed_command = command @@ -78,8 +78,8 @@ def make_session(tmp_path): ), runtime=str(tmp_path / "runtime"), ) - shutil.copyfile(sandbox_runner.__file__, directory / "sandbox_runner.py") - assert not (directory / "process_supervisor.py").exists() + shutil.copyfile(sandbox_runner.__file__, directory / "sandbox_runner") + assert not (directory / "process_supervisor").exists() return state, provider, workdir @@ -154,6 +154,30 @@ async def test_exec_only_supervision_leaves_task_children_for_sandbox_owner(tmp_ pass +async def test_supervisor_and_runner_run_under_neutral_names(tmp_path): + """A task agent's `pkill -f python3` or `killall python3` must not match the processes above OpenClaw.""" + state, provider, workdir = make_session(tmp_path) + code = "import pathlib,time; pathlib.Path('ready').touch(); time.sleep(60)" + task = state.task = asyncio.create_task( + state.execute(payload(state, code, timeout=60), timeout=60, close_timeout=3) + ) + try: + async with asyncio.timeout(5): + while not (workdir / "ready").exists(): + await asyncio.sleep(0.01) + supervisor_pid = int((Path(state.session.session_dir) / "supervisor.pid").read_text()) + (runner_pid,) = map(int, Path(f"/proc/{supervisor_pid}/task/{supervisor_pid}/children").read_text().split()) + for pid, name in ((supervisor_pid, "gym-supervisor"), (runner_pid, "gym-runner")): + assert Path(f"/proc/{pid}/comm").read_text().strip() == name + cmdline = Path(f"/proc/{pid}/cmdline").read_bytes().replace(b"\0", b" ").decode() + assert "python" not in cmdline and ".py" not in cmdline, cmdline + finally: + await state.close(3) + await asyncio.gather(task, return_exceptions=True) + assert state.session.cleanup["cleanup_confirmed"] is True + assert provider.disconnected + + async def test_lost_launch_is_fenced_even_after_directory_retirement(tmp_path): state, provider, workdir = make_session(tmp_path) provider.lost_launch = True diff --git a/responses_api_agents/openclaw_agent/tests/test_sandbox.py b/responses_api_agents/openclaw_agent/tests/test_sandbox.py index 9971820d60c..241675f15c8 100644 --- a/responses_api_agents/openclaw_agent/tests/test_sandbox.py +++ b/responses_api_agents/openclaw_agent/tests/test_sandbox.py @@ -116,7 +116,7 @@ def pty(self): raise AssertionError("This provider supports exec only; no PTY API") async def run_exec(self, command, **kwargs): - if "--receipt" in command and "process_supervisor.py" in command: + if "--receipt" in command and "process_supervisor" in command: return await self.launch(command=command, **kwargs) if "stop.request" in command: await self.request_stop() @@ -133,7 +133,7 @@ async def create(self, **kwargs): payload = json.loads(self.files[payload_path]) assert payload["cwd"] == getattr(self, "expected_workdir", "/app") assert kwargs["cwd"] == getattr(self, "expected_workdir", "/app") - assert "sandbox_runner.py" in kwargs["command"] + assert "/sandbox_runner " in kwargs["command"] and "/bin/gym-runner" in kwargs["command"] self.directory = payload["directory"] self.started.set() if not self.blocked: @@ -1897,11 +1897,11 @@ async def test_close_retains_capture_after_provider_release(setup, tmp_path, own await agent.seed_agent_session(request, body) state = agent._session_records[body.agent_session_id].state directory = state.session.session_dir - assert f"{directory}/process_supervisor.py" not in sandbox.files + assert f"{directory}/process_supervisor" not in sandbox.files sandbox.blocked = True task = asyncio.create_task(agent.responses(request, NeMoGymResponseCreateParamsNonStreaming(input="task"))) await asyncio.wait_for(sandbox.started.wait(), 2) - assert f"{directory}/process_supervisor.py" in sandbox.files + assert f"{directory}/process_supervisor" in sandbox.files async def release(): assert state.session.cleanup["cleanup_confirmed"] is True diff --git a/responses_api_agents/opencode_agent/README.md b/responses_api_agents/opencode_agent/README.md index b9a895f7d10..695ec549405 100644 --- a/responses_api_agents/opencode_agent/README.md +++ b/responses_api_agents/opencode_agent/README.md @@ -94,6 +94,9 @@ tools in separate sessions. The sandbox owner stops the preserved services later Natural CLI exit cannot distinguish unfinished tools in other sessions from services; those processes may still change task files. The stop-time cutoff also has adoption/PID races and cannot classify later service forks. It is not a security boundary. +The supervisor and the runner above OpenCode run as `gym-supervisor` and `gym-runner` (links to the +sandbox's `python3`) from files without a `.py` suffix, so a task agent's `pkill -f python3`, +`pkill python3` or `killall python3` does not end OpenCode. The CLI must exit before SQLite observations are captured; services must use task-owned files rather than the agent session directory, which close removes. Capture requires confirmed termination. diff --git a/responses_api_agents/opencode_agent/app.py b/responses_api_agents/opencode_agent/app.py index 3ca098c5651..4bc29f45c4a 100644 --- a/responses_api_agents/opencode_agent/app.py +++ b/responses_api_agents/opencode_agent/app.py @@ -37,7 +37,7 @@ AgentContinuationCapabilities, ) from nemo_gym.agent_outcome import AgentOutcome -from nemo_gym.agent_utils.sandbox_session import SandboxSession +from nemo_gym.agent_utils.sandbox_session import RUNNER_FILE, SandboxSession from nemo_gym.agent_utils.task_timeout import AGENT_TIMEOUT_METADATA_KEY, resolve_agent_timeout from nemo_gym.base_resources_server import BaseRunRequest, BaseVerifyResponse from nemo_gym.base_responses_api_agent import ( @@ -766,7 +766,7 @@ async def _seed_agent_session_state(self, body: AgentSeedSessionRequest) -> Open ) result = await sandbox.exec(command, cwd=workdir, timeout_s=self.config.sandbox_install_timeout_seconds) self._check_sandbox_setup(command, result) - await sandbox.upload(Path(__file__).with_name("sandbox_runner.py"), f"{directory}/sandbox_runner.py") + await sandbox.upload(Path(__file__).with_name("sandbox_runner.py"), f"{directory}/{RUNNER_FILE}") await sandbox.upload(Path(__file__).with_name("task_environment.mjs"), f"{directory}/task_environment.mjs") if self._model_call_capture_enabled(): await sandbox.upload(OBSERVABILITY_PATCH, f"{directory}/{OBSERVABILITY_PATCH.name}") diff --git a/responses_api_agents/opencode_agent/sandbox.py b/responses_api_agents/opencode_agent/sandbox.py index cea5377da86..ea5de91ccd2 100644 --- a/responses_api_agents/opencode_agent/sandbox.py +++ b/responses_api_agents/opencode_agent/sandbox.py @@ -12,7 +12,7 @@ from pydantic import BaseModel, ConfigDict, JsonValue, ValidationError -from nemo_gym.agent_utils.sandbox_session import SandboxCommand, SandboxSession +from nemo_gym.agent_utils.sandbox_session import RUNNER_FILE, SandboxCommand, SandboxSession from nemo_gym.base_responses_api_agent import AgentSessionState from nemo_gym.config_types import ModelServerRef from nemo_gym.openai_utils import NeMoGymResponse, NeMoGymResponseCreateParamsNonStreaming @@ -239,9 +239,10 @@ async def stage_activation(self, payload: dict[str, JsonValue]) -> SandboxComman argv=[ self.python_executable, "-I", - f"{self.persistent_directory}/sandbox_runner.py", + f"{self.persistent_directory}/{RUNNER_FILE}", f"{self.session.session_dir}/input.json", ], + stdlib_runner=True, ) async def collect_artifacts(self, *, timeout: float) -> str: @@ -369,7 +370,7 @@ async def snapshot(self, timeout: float) -> None: if self.session.cleanup is None or not self.session.cleanup["cleanup_confirmed"]: raise RuntimeError("OpenCode transcript capture requires confirmed cleanup") captured = await self.session.sandbox.exec( - f"{quote(self.python_executable)} -I {quote(self.persistent_directory + '/sandbox_runner.py')} " + f"{quote(self.python_executable)} -I {quote(f'{self.persistent_directory}/{RUNNER_FILE}')} " f"--snapshot {quote(self.session.session_dir)} {quote(self.persistent_directory)}", cwd=self.session.workdir, timeout_s=timeout, diff --git a/responses_api_agents/opencode_agent/tests/test_continuation.py b/responses_api_agents/opencode_agent/tests/test_continuation.py index 958a28192f4..7347eaa29df 100644 --- a/responses_api_agents/opencode_agent/tests/test_continuation.py +++ b/responses_api_agents/opencode_agent/tests/test_continuation.py @@ -14,6 +14,7 @@ from fastapi.testclient import TestClient from nemo_gym.agent_activation_types import AgentContinuationRequirements +from nemo_gym.agent_utils.supervisor_client import SUPERVISOR_FILE from nemo_gym.base_responses_api_agent import AGENT_ACTIVATION_HEADER, AgentCloseSessionRequest from nemo_gym.openai_utils import NeMoGymResponseCreateParamsNonStreaming from responses_api_agents.opencode_agent.continuation import classify_opencode_run @@ -413,7 +414,7 @@ async def slow_exec(command, **kwargs): return execute.return_value async def slow_upload(source, destination): - if stage == "upload" and destination.endswith("process_supervisor.py"): + if stage == "upload" and destination.endswith(f"/{SUPERVISOR_FILE}"): await asyncio.sleep(0.2) await upload(source, destination) diff --git a/responses_api_agents/opencode_agent/tests/test_exec_transport.py b/responses_api_agents/opencode_agent/tests/test_exec_transport.py index 289dbe2046d..6799a3cfa62 100644 --- a/responses_api_agents/opencode_agent/tests/test_exec_transport.py +++ b/responses_api_agents/opencode_agent/tests/test_exec_transport.py @@ -49,7 +49,7 @@ async def exec(self, command, *, cwd=None, timeout_s=30): if self.worker_pid is not None: with pytest.raises(ProcessLookupError): os.kill(int(self.worker_pid.read_text()), 0) - launch = "--receipt" in command and "process_supervisor.py" in command + launch = "--receipt" in command and "process_supervisor" in command if launch: self.deadline = timeout_s self.delayed_command = command @@ -86,8 +86,8 @@ def make_session(tmp_path): ), runtime=str(tmp_path / "runtime"), ) - shutil.copyfile(sandbox_runner.__file__, directory / "sandbox_runner.py") - assert not (directory / "process_supervisor.py").exists() + shutil.copyfile(sandbox_runner.__file__, directory / "sandbox_runner") + assert not (directory / "process_supervisor").exists() database = directory / "data/opencode/opencode.db" database.parent.mkdir(parents=True) with sqlite3.connect(database) as connection: @@ -177,6 +177,34 @@ async def test_exec_only_supervision_leaves_task_children_for_sandbox_owner(tmp_ pass +async def test_supervisor_and_runner_run_under_neutral_names(tmp_path): + """A task agent's `pkill -f python3` or `killall python3` must not match the processes above OpenCode.""" + state, provider, workdir = make_session(tmp_path) + code = ( + "import pathlib,time; " + f"pathlib.Path({state.session.session_dir!r},'task-environment.ready').write_text('ready\\n'); " + "pathlib.Path('ready').touch(); time.sleep(60)" + ) + task = state.task = asyncio.create_task( + state.execute(payload(state, code, timeout=60), timeout=60, close_timeout=3) + ) + try: + async with asyncio.timeout(5): + while not (workdir / "ready").exists(): + await asyncio.sleep(0.01) + supervisor_pid = int((Path(state.session.session_dir) / "supervisor.pid").read_text()) + (runner_pid,) = map(int, Path(f"/proc/{supervisor_pid}/task/{supervisor_pid}/children").read_text().split()) + for pid, name in ((supervisor_pid, "gym-supervisor"), (runner_pid, "gym-runner")): + assert Path(f"/proc/{pid}/comm").read_text().strip() == name + cmdline = Path(f"/proc/{pid}/cmdline").read_bytes().replace(b"\0", b" ").decode() + assert "python" not in cmdline and ".py" not in cmdline, cmdline + finally: + await state.close(3) + await asyncio.gather(task, return_exceptions=True) + assert state.session.cleanup["cleanup_confirmed"] is True + assert provider.disconnected + + async def test_lost_launch_is_fenced_even_after_directory_retirement(tmp_path): state, provider, workdir = make_session(tmp_path) provider.lost_launch = True diff --git a/responses_api_agents/opencode_agent/tests/test_sandbox_sessions.py b/responses_api_agents/opencode_agent/tests/test_sandbox_sessions.py index 927ec21f492..e1805eb4b0b 100644 --- a/responses_api_agents/opencode_agent/tests/test_sandbox_sessions.py +++ b/responses_api_agents/opencode_agent/tests/test_sandbox_sessions.py @@ -122,7 +122,7 @@ def pty(self): raise AssertionError("This provider supports exec only; no PTY API") async def run_exec(self, command, **kwargs): - if "--receipt" in command and "process_supervisor.py" in command: + if "--receipt" in command and "process_supervisor" in command: return await self.launch(command=command, **kwargs) if "stop.request" in command: await self.request_stop() @@ -139,7 +139,7 @@ async def create(self, **kwargs): payload = json.loads(self.files[payload_path]) assert payload["cwd"] == getattr(self, "expected_workdir", "/app") assert kwargs["cwd"] == getattr(self, "expected_workdir", "/app") - assert "sandbox_runner.py" in kwargs["command"] + assert "/sandbox_runner " in kwargs["command"] and "/bin/gym-runner" in kwargs["command"] self.directory = payload["directory"] self.started.set() if not self.blocked: @@ -1429,11 +1429,11 @@ async def test_close_keeps_captured_events_after_sandbox_release(setup, tmp_path await agent.seed_agent_session(request, body) state = agent._session_records[body.agent_session_id].state directory = state.session.session_dir - assert f"{directory}/process_supervisor.py" not in sandbox.files + assert f"{directory}/process_supervisor" not in sandbox.files sandbox.blocked = True task = asyncio.create_task(agent.responses(request, NeMoGymResponseCreateParamsNonStreaming(input="task"))) await asyncio.wait_for(sandbox.started.wait(), 2) - assert f"{directory}/process_supervisor.py" in sandbox.files + assert f"{directory}/process_supervisor" in sandbox.files async def release(): assert state.session.cleanup["cleanup_confirmed"] is True diff --git a/responses_api_agents/pi_agent/README.md b/responses_api_agents/pi_agent/README.md index 082f254980c..f91fbc4251d 100644 --- a/responses_api_agents/pi_agent/README.md +++ b/responses_api_agents/pi_agent/README.md @@ -172,6 +172,9 @@ Natural CLI exit cannot distinguish unfinished tools in other sessions from serv those processes may still change task files. The stop-time cutoff also has adoption/PID races and cannot classify later service forks. It is not a security boundary. Services must use task-owned files rather than the temporary agent session directory. +The supervisor and the runner above Pi run as `gym-supervisor` and `gym-runner` (links to the +sandbox's `python3`) from files without a `.py` suffix, so a task agent's `pkill -f python3`, +`pkill python3` or `killall python3` does not end Pi. Harness execution uses `nemo_gym.agent_utils.sandbox_session.SandboxSession`, shared with Hermes. The adapter stages its worker input and collects its own output. The shared session diff --git a/responses_api_agents/pi_agent/app.py b/responses_api_agents/pi_agent/app.py index 0ca94d8ae69..11fc0d1aed4 100644 --- a/responses_api_agents/pi_agent/app.py +++ b/responses_api_agents/pi_agent/app.py @@ -32,7 +32,7 @@ from pydantic import BaseModel, ConfigDict, Field, PrivateAttr from nemo_gym.agent_outcome import AgentOutcome -from nemo_gym.agent_utils.sandbox_session import SandboxSession +from nemo_gym.agent_utils.sandbox_session import RUNNER_FILE, SandboxSession from nemo_gym.agent_utils.task_timeout import AGENT_TIMEOUT_METADATA_KEY, resolve_agent_timeout from nemo_gym.base_resources_server import BaseRunRequest, BaseVerifyResponse from nemo_gym.base_responses_api_agent import ( @@ -733,7 +733,7 @@ async def _seed_agent_session_state(self, body: AgentSeedSessionRequest) -> PiSa f"Pi sandbox installation failed (exit {installed.return_code}, " f"error_type={installed.error_type}): {details[-16000:]}" ) - await sandbox.upload(Path(__file__).with_name("sandbox_runner.py"), f"{directory}/sandbox_runner.py") + await sandbox.upload(Path(__file__).with_name("sandbox_runner.py"), f"{directory}/{RUNNER_FILE}") except BaseException as error: try: await state.close(self.config.session_close_timeout_seconds) diff --git a/responses_api_agents/pi_agent/sandbox.py b/responses_api_agents/pi_agent/sandbox.py index 3ef322409c0..96bbe1d3c48 100644 --- a/responses_api_agents/pi_agent/sandbox.py +++ b/responses_api_agents/pi_agent/sandbox.py @@ -9,7 +9,7 @@ from pydantic import BaseModel, ConfigDict, JsonValue, ValidationError -from nemo_gym.agent_utils.sandbox_session import SandboxCommand, SandboxSession +from nemo_gym.agent_utils.sandbox_session import RUNNER_FILE, SandboxCommand, SandboxSession from nemo_gym.base_responses_api_agent import AgentSessionState from nemo_gym.openai_utils import NeMoGymResponse, NeMoGymResponseCreateParamsNonStreaming from nemo_gym.rollout_observability import AgentObservationBundle @@ -89,9 +89,10 @@ async def stage_activation(self, payload: dict[str, JsonValue]) -> SandboxComman argv=[ "python3", "-I", - f"{self.session.session_dir}/sandbox_runner.py", + f"{self.session.session_dir}/{RUNNER_FILE}", f"{self.session.session_dir}/input.json", ], + stdlib_runner=True, ) async def collect_artifacts(self) -> str: diff --git a/responses_api_agents/pi_agent/tests/test_sandbox.py b/responses_api_agents/pi_agent/tests/test_sandbox.py index bcf92ce4264..0c090421740 100644 --- a/responses_api_agents/pi_agent/tests/test_sandbox.py +++ b/responses_api_agents/pi_agent/tests/test_sandbox.py @@ -117,7 +117,7 @@ def __init__(self): self.cleanup_available = True async def execute(self, command, **kwargs): - if "--receipt" in command and "process_supervisor.py" in command: + if "--receipt" in command and "process_supervisor" in command: return await self.launch(command, **kwargs) if "ln -s stop " in command: await self.signal() @@ -143,8 +143,8 @@ async def run(self, command, **kwargs): payload = json.loads(self.files[payload_path]) assert payload["cwd"] == getattr(self, "expected_workdir", "/app") assert kwargs["cwd"] == getattr(self, "expected_workdir", "/app") - assert "sandbox_runner.py" in command - assert "process_supervisor.py" in command + assert "/sandbox_runner " in command and "/bin/gym-runner" in command + assert "process_supervisor" in command self.directory = payload["directory"] self.started.set() if not self.blocked: @@ -216,7 +216,7 @@ def test_http_session_flow_runs_pi_in_borrowed_sandbox(setup): assert created.status_code == 200, created.text session_id = created.json()["agent_session_id"] directory = agent._session_records[session_id].state.session.session_dir - assert f"{directory}/process_supervisor.py" not in sandbox.files + assert f"{directory}/process_supervisor" not in sandbox.files installer = f"{directory}/install_pi_runtime.sh" assert installer in sandbox.files assert agent.config.resources_server is None @@ -227,7 +227,7 @@ def test_http_session_flow_runs_pi_in_borrowed_sandbox(setup): assert install_call.kwargs["timeout_s"] == agent.config.sandbox_install_timeout_seconds result = client.post("/ng-rollout/pi-smoke-a2/v1/responses", json={"input": "Fix the code"}) assert result.status_code == 200, result.text - assert f"{directory}/process_supervisor.py" in sandbox.files + assert f"{directory}/process_supervisor" in sandbox.files body = result.json() assert body["status"] == "completed" assert [item["type"] for item in body["output"]] == [ @@ -1481,7 +1481,7 @@ def _run_session_locally(sandbox, state, directory: Path, *, limit: float = 8) - from responses_api_agents.pi_agent import sandbox_runner state.session.session_dir = str(directory) - shutil.copyfile(sandbox_runner.__file__, directory / "sandbox_runner.py") + shutil.copyfile(sandbox_runner.__file__, directory / "sandbox_runner") sandbox.upload = AsyncMock(side_effect=shutil.copyfile) sandbox.download = AsyncMock(side_effect=shutil.copyfile) sandbox.exec.side_effect = functools.partial(_run_locally, limit=limit) @@ -1509,7 +1509,7 @@ async def test_close_fences_delayed_launch_before_and_after_removing_session(loc commands = [] async def queued_exec(command, **kwargs): - if "--receipt" in command and "process_supervisor.py" in command: + if "--receipt" in command and "process_supervisor" in command: commands.append(command) waiting.set() if failure == "cancel": diff --git a/tests/unit_tests/test_sandbox_session.py b/tests/unit_tests/test_sandbox_session.py index be69612776b..5d19fb22666 100644 --- a/tests/unit_tests/test_sandbox_session.py +++ b/tests/unit_tests/test_sandbox_session.py @@ -192,7 +192,8 @@ async def test_rearm_uses_fresh_launch_files_and_close_removes_the_root(session) first.assert_awaited_once() second.assert_awaited_once() uploads = [c.args[1] for c in session.sandbox.upload.await_args_list] - assert uploads == ["/session/activation-1/process_supervisor.py", "/session/activation-2/process_supervisor.py"] + supervisor = supervisor_client.SUPERVISOR_FILE + assert uploads == [f"/session/activation-1/{supervisor}", f"/session/activation-2/{supervisor}"] await session.close(timeout=1) assert "mv /session /session.closed" in session.sandbox.exec.await_args.args[0] with pytest.raises(RuntimeError, match="closing"): diff --git a/tests/unit_tests/test_sandbox_supervisor_bootstrap.py b/tests/unit_tests/test_sandbox_supervisor_bootstrap.py index 015d48b79b8..9ea3043fa0b 100644 --- a/tests/unit_tests/test_sandbox_supervisor_bootstrap.py +++ b/tests/unit_tests/test_sandbox_supervisor_bootstrap.py @@ -7,14 +7,18 @@ import shutil import sys from pathlib import Path +from shlex import quote import pytest from nemo_gym.agent_utils import process_supervisor +from nemo_gym.agent_utils.sandbox_session import SandboxCommand from nemo_gym.agent_utils.supervisor_client import ( CLEANUP_RECEIPT_FILE, LAUNCH_CLAIM_FILE, + RUNNER_INTERPRETER, SUPERVISOR_FILE, + SUPERVISOR_INTERPRETER, SUPERVISOR_PID_FILE, remove_session_directory, stop_and_confirm_cleanup, @@ -54,7 +58,7 @@ async def test_bootstrap_failure_preserves_diagnostics_and_allows_close(tmp_path expected_error = "incompatible interpreter" elif failure == "missing_supervisor": supervisor.unlink() - expected_error = "process_supervisor.py" + expected_error = f"session/{SUPERVISOR_FILE}" else: supervisor.write_text("def invalid syntax\n") expected_error = "SyntaxError" @@ -154,9 +158,55 @@ async def test_close_during_preflight_fences_later_launch(tmp_path: Path, remove assert not worker_started.exists() -async def test_bootstrap_quotes_private_paths_and_uses_the_same_interpreter(tmp_path: Path) -> None: +async def test_launch_delayed_before_linking_does_not_recreate_a_removed_session(tmp_path: Path) -> None: + session_dir = tmp_path / "session" + session_dir.mkdir() + (session_dir / SUPERVISOR_FILE).write_text("") + resolving = tmp_path / "resolving" + resume = tmp_path / "resume" + python = tmp_path / "python" + python.write_text( + "#!/bin/sh\n" + f'if [ "$2" = -c ]; then touch {quote(str(resolving))}; ' + f"while [ ! -e {quote(str(resume))} ]; do sleep 0.01; done; fi\n" + f'exec {quote(sys.executable)} "$@"\n' + ) + python.chmod(0o755) + sandbox = LocalSandbox() + launch = asyncio.create_task( + sandbox.exec( + supervised_launch_command( + session_dir=str(session_dir), command=["worker"], timeout=1, cleanup_timeout=1, python=str(python) + ) + ) + ) + try: + async with asyncio.timeout(5): + while not resolving.exists(): + await asyncio.sleep(0.01) + await stop_and_confirm_cleanup( + sandbox, session_dir=str(session_dir), workdir=None, timeout=1, harness="bootstrap-test" + ) + await remove_session_directory( + sandbox, session_dir=str(session_dir), workdir=None, timeout=1, harness="bootstrap-test" + ) + finally: + resume.touch() + await asyncio.wait_for(launch, timeout=5) + assert not session_dir.exists() + + +@pytest.mark.parametrize("stdlib_runner", [False, True]) +@pytest.mark.parametrize("linkable", [True, False]) +async def test_bootstrap_quotes_private_paths_and_uses_the_same_interpreter( + tmp_path: Path, linkable: bool, stdlib_runner: bool +) -> None: + # Interpreter links resolve /proc/self/exe; elsewhere the launch falls back to the given interpreter. + linkable = linkable and sys.platform == "linux" session_dir = tmp_path / "session's $(touch unintended) files" session_dir.mkdir() + if not linkable: + (session_dir / "bin").write_text("not a directory") # The launch must fall back to the given interpreter. runtime = tmp_path / "runtime's $(touch unintended) files" runtime.mkdir() python = runtime / "python interpreter" @@ -170,15 +220,47 @@ async def test_bootstrap_quotes_private_paths_and_uses_the_same_interpreter(tmp_ ) command = supervised_launch_command( session_dir=str(session_dir), - command=["worker", "$(touch unintended)", "argument's value"], + command=[str(python), "worker", "$(touch unintended)", "argument's value"], timeout=1, cleanup_timeout=1, python=str(python), + stdlib_runner=stdlib_runner, ) result = await LocalSandbox().exec(command, cwd=str(tmp_path)) assert result.return_code == 0, result.stderr captured = json.loads(arguments.read_text()) - assert captured[0] == str(python) - assert captured[-3:] == ["worker", "$(touch unintended)", "argument's value"] + supervisor_python = session_dir / SUPERVISOR_INTERPRETER if linkable else python + runner_python = session_dir / RUNNER_INTERPRETER if linkable and stdlib_runner else python + assert captured[0] == str(supervisor_python) + assert captured[captured.index("--") + 1 :] == [ + str(runner_python), + "worker", + "$(touch unintended)", + "argument's value", + ] + assert supervisor_python.resolve() == runner_python.resolve() == python.resolve() assert not (tmp_path / "unintended").exists() assert (session_dir / LAUNCH_CLAIM_FILE).readlink() == Path("launch") + + +@pytest.mark.parametrize("argv", [["/runtime/venv/bin/python", "runner"], []]) +def test_stdlib_runner_rejects_an_argv_that_does_not_start_with_the_interpreter(argv: list[str]) -> None: + with pytest.raises(ValueError, match="stdlib_runner requires"): + supervised_launch_command( + session_dir="/session", command=argv, timeout=1, cleanup_timeout=1, python="python3", stdlib_runner=True + ) + with pytest.raises(ValueError, match="stdlib_runner requires"): + SandboxCommand(argv=argv, python="python3", stdlib_runner=True) + + +def test_other_harness_commands_keep_their_interpreter() -> None: + command = supervised_launch_command( + session_dir="/session", + command=["/runtime/venv/bin/python", "runner"], + timeout=1, + cleanup_timeout=1, + python="python3", + ) + assert command.endswith("-- /runtime/venv/bin/python runner >/session/output.log 2>&1") + assert RUNNER_INTERPRETER not in command + assert SandboxCommand(argv=["/runtime/venv/bin/python", "runner"], python="python3").stdlib_runner is False diff --git a/tests/unit_tests/test_sandbox_supervisor_client.py b/tests/unit_tests/test_sandbox_supervisor_client.py index 8adadd1fd6f..3dab5b74192 100644 --- a/tests/unit_tests/test_sandbox_supervisor_client.py +++ b/tests/unit_tests/test_sandbox_supervisor_client.py @@ -5,6 +5,7 @@ import asyncio import json import shutil +import subprocess import sys from pathlib import Path from unittest.mock import AsyncMock @@ -174,6 +175,73 @@ async def test_launch_supervises_harness_with_selected_or_private_python(session assert set((session_dir / OUTPUT_LOG_FILE).read_text().splitlines()) == {"$(literal)", "stderr"} +# Starts a non-Python tool, like an agent CLI, and waits for it. +_RUNNER = """ +import pathlib, subprocess, sys +tool = subprocess.Popen(["sleep", "60"]) +pathlib.Path(sys.argv[1]).write_text(str(tool.pid)) +tool.wait() +""" + + +def _alive(pid: int) -> bool: + try: + return Path(f"/proc/{pid}/stat").read_text().split(") ")[-1][0] != "Z" + except FileNotFoundError: + return False + + +@pytest.mark.skipif(sys.platform != "linux" or shutil.which("pgrep") is None, reason="Linux /proc and pgrep") +async def test_kills_by_interpreter_name_miss_the_supervisor_and_stdlib_runner(session, tmp_path): + """pgrep matches exactly as pkill and killall would, without signalling anything on this host.""" + sandbox, session_dir = session + shutil.copyfile(process_supervisor.__file__, session_dir / SUPERVISOR_FILE) + runner = session_dir / "sandbox_runner" + runner.write_text(_RUNNER) + tool_pid_file = session_dir / "tool.pid" + command = supervised_launch_command( + session_dir=str(session_dir), + command=[sys.executable, "-I", str(runner), str(tool_pid_file)], + timeout=60, + cleanup_timeout=1, + python=sys.executable, + stdlib_runner=True, + ) + launch = asyncio.create_task(sandbox.exec(command, timeout_s=90)) + try: + async with asyncio.timeout(10): + while not tool_pid_file.exists() or not tool_pid_file.read_text(): + await asyncio.sleep(0.01) + supervisor_pid = int((session_dir / SUPERVISOR_PID_FILE).read_text()) + (runner_pid,) = map(int, Path(f"/proc/{supervisor_pid}/task/{supervisor_pid}/children").read_text().split()) + tool_pid = int(tool_pid_file.read_text()) + for pid, name in ((supervisor_pid, "gym-supervisor"), (runner_pid, "gym-runner")): + assert Path(f"/proc/{pid}/comm").read_text().strip() == name + cmdline = Path(f"/proc/{pid}/cmdline").read_bytes().replace(b"\0", b" ").decode() + assert "python" not in cmdline and ".py" not in cmdline, cmdline + assert Path(f"/proc/{pid}/exe").resolve() == Path(sys.executable).resolve() + # pkill -f python3 / python / "python3 -" / "\.py"; pkill python; killall python3 (an exact name). + for args in ( + ["-f", "python3"], + ["-f", "python"], + ["-f", "python3 -"], + ["-f", r"\.py"], + ["python"], + ["-x", "python3"], + ): + matched = {int(pid) for pid in subprocess.run(["pgrep", *args], capture_output=True).stdout.split()} + assert not matched & {supervisor_pid, runner_pid}, args + assert all(map(_alive, (supervisor_pid, runner_pid, tool_pid))) + finally: + # Gym's own stop signals the recorded PID, whatever the process is called. + receipt = await stop_and_confirm_cleanup( + sandbox, session_dir=str(session_dir), workdir=str(tmp_path), timeout=5, harness="test" + ) + await asyncio.wait_for(launch, timeout=10) + assert receipt["cleanup_confirmed"] is True and receipt["error"] is None + assert not any(map(_alive, (supervisor_pid, runner_pid, tool_pid))) + + @pytest.mark.parametrize("confirmed", [False, "true", 1]) async def test_unconfirmed_cleanup_preserves_receipt_for_retry(session, confirmed): sandbox, session_dir = session