From 4dfb5a46fecbd596df9be8a66f59e4419a55d349 Mon Sep 17 00:00:00 2001
From: Ovtcharov <kovtchar@amd.com>
Date: Sat, 5 Sep 2026 03:40:43 -0700
Subject: [PATCH 1/4] feat(shell): add a shell session whose cwd and
 environment persist
Ports the C++ toolbelt's ShellSession (cpp/include/gaia/process.h, #2810) to
Python. What persists is the session state, not a resident child: each command
runs a generated script that restores the session's directory and variables,
sources the command from its own file, then reports the resulting pwd and
environment to a side file the session absorbs.
Not wired into the shell tool yet.
---
 src/gaia/agents/tools/shell_session.py &#124; 657 +++++++++++++++++++++++++
 tests/unit/test_shell_session.py &#124; 253 ++++++++++
 2 files changed, 910 insertions(+)
 create mode 100644 src/gaia/agents/tools/shell_session.py
 create mode 100644 tests/unit/test_shell_session.py
diff --git a/src/gaia/agents/tools/shell_session.py b/src/gaia/agents/tools/shell_session.py
new file mode 100644
index 000000000..e6b47b8d3
--- /dev/null
+++ b/src/gaia/agents/tools/shell_session.py
@@ -0,0 +1,657 @@
+# Copyright(C) 2025-2026 Advanced Micro Devices, Inc. All rights reserved.
+# SPDX-License-Identifier: MIT
+"""A shell whose working directory and environment survive between commands.
+
+One-shot execution is materially worse than a human terminal for the
+build/test loop an agent spends most of its time in: ``cd build`` in one call
+is invisible to the next, and so is ``export CC=clang`` or a virtualenv
+activation. No system prompt can fix that — the state simply is not there to
+observe. A :class:`ShellSession` keeps it.
+
+This is a port of the C++ toolbelt's ``gaia::ShellSession``
+(``cpp/include/gaia/process.h``, #2810). What persists is the *session state*,
+not a resident child process: each call runs a generated script that restores
+the session's cwd and variables, sources the command from its own file, then
+writes the resulting ``pwd`` and environment to a side file that the session
+absorbs. A resident shell driven over pipes would have to survive a timeout to
+be worth anything, and a timed-out command that is still inside a shared shell
+is exactly the corruption this is meant to avoid — so there is no long-lived
+child to orphan, on any platform.
+
+The security model is unchanged. This class only preserves state; it does not
+decide what may run. Command validation stays with the caller and applies per
+command exactly as before.
+
+Two further properties of the generated script are worth knowing:
+
+- stdin is ``/dev/null``. An interactive command (``git commit`` opening an
+ editor, ``sudo``, ``npm login``) returns immediately instead of sitting until
+ the timeout, because a tool call has no way to answer it.
+- On timeout the whole process *group* is killed, not just the shell — a build
+ or test command spawns children, and leaving them running is what makes a
+ timed-out build worse than useless.
+
+Windows without a POSIX shell: commands run as a ``cmd.exe`` batch script,
+because keeping ``cd`` and ``set`` requires running in the same interpreter and
+cmd only offers that to a script. ``%VAR%`` expansion is identical to a prompt,
+but a ``for`` loop variable is written ``%%i`` rather than ``%i``. Point
+``GAIA_SHELL`` at a POSIX shell (Git Bash, MSYS, WSL) and none of this applies.
+"""
+
+import locale
+import logging
+import os
+import re
+import shutil
+import signal
+import subprocess # nosec B404 - running shell commands is this module's purpose
+import tempfile
+import threading
+import time
+from dataclasses import dataclass, field
+from pathlib import Path
+from typing import Callable, Dict, List, Optional
+
+logger = logging.getLogger(__name__)
+
+#: Separates the reported cwd from the environment dump in the state file.
+_ENV_MARKER = "---GAIA-ENV---"
+
+#: Variables every shell rewrites on its own. Replaying them would make the
+#: session drift a little further from the parent on every command.
+_VOLATILE_ENV_NAMES = frozenset(
+ {
+ "_",
+ "PWD",
+ "OLDPWD",
+ "SHLVL",
+ "PS1",
+ "PS2",
+ "RANDOM",
+ "SECONDS",
+ "LINENO",
+ "PROMPT",
+ "CD",
+ "ERRORLEVEL",
+ "CMDCMDLINE",
+ "CMDEXTVERSION",
+ "__GAIA_RC",
+ }
+)
+
+_VALID_ENV_NAME = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$")
+
+
+def _env_key(name: str) -> str:
+ """The name to compare environment variables under.
+
+ Windows variable names are case-insensitive and ``os.environ`` upper-cases
+ them, while ``set`` reports the original casing. Comparing raw would make
+ every inherited variable look changed *and* look removed, so the session
+ would replay the whole environment and unset it at the same time.
+ """
+ return name.upper() if os.name == "nt" else name
+
+
+#: How long to wait for a killed process tree to release its pipes.
+_DRAIN_TIMEOUT_SECONDS = 5.0
+
+#: How long to give the platform's tree-killer before giving up on it.
+_TREE_KILL_TIMEOUT_SECONDS = 15.0
+
+
+class ShellSessionError(RuntimeError):
+ """Base for the errors a session raises instead of degrading quietly."""
+
+
+class ShellSessionBusy(ShellSessionError):
+ """Another command still holds the session.
+
+ A tool call that exceeds its timeout leaves its worker thread running
+ (#2600), so a previous command can still be inside the subprocess when the
+ next one arrives. Blocking forever would wedge the agent; this says so.
+ """
+
+
+class ShellSessionClosed(ShellSessionError):
+ """The session was torn down and will not run anything else."""
+
+
+@dataclass
+class ShellResult:
+ """What one command in a session produced."""
+
+ stdout: str = ""
+ stderr: str = ""
+ return_code: int = 0
+ timed_out: bool = False
+ duration_seconds: float = 0.0
+ cwd: str = ""
+ #: Set when the command's ``cd`` was refused by the session's cwd guard.
+ cwd_change_rejected: Optional[str] = None
+
+
+@dataclass
+class _State:
+ """The cwd and environment divergence the session carries forward."""
+
+ cwd: str
+ overrides: Dict[str, str] = field(default_factory=dict)
+ unset: set = field(default_factory=set)
+
+
+def _is_valid_env_name(name: str) -> bool:
+ return bool(_VALID_ENV_NAME.match(name))
+
+
+def _posix_quote(value: str) -> str:
+ """Wrap *value* in single quotes for POSIX sh."""
+ return "'" + value.replace("'", "'\\''") + "'"
+
+
+def _batch_quote(value: str) -> str:
+ """Escape *value* for use inside a batch file.
+
+ Only ``%`` needs escaping. ``"`` must be left alone: ``set "K=V"`` takes
+ everything up to the *last* quote on the line, so doubling a quote is not
+ undone — and because the session re-captures and re-emits the value, every
+ command would double it again until the line broke.
+ """
+ return value.replace("%", "%%")
+
+
+def _generic_path(path: str) -> str:
+ """Forward-slash form, which every POSIX shell accepts — Git Bash included."""
+ return Path(path).as_posix()
+
+
+def _split_env_record(record: str, out: Dict[str, str]) -> None:
+ """Split one ``NAME=VALUE`` record.
+
+ Names are not validated here: ``ProgramFiles(x86)`` is a real Windows
+ variable, and mis-parsing it would corrupt the neighbour it sorts next to.
+ """
+ name, sep, value = record.partition("=")
+ if not sep or not name:
+ return
+ out[name] = value
+
+
+def _parse_env_records_nul(text: str) -> Dict[str, str]:
+ """Parse the NUL-delimited environment dump the POSIX script emits.
+
+ NUL is the one byte an environment value cannot contain, so the record
+ boundary is unambiguous. Line-delimited ``env`` output is not: a value
+ containing a newline followed by ``SOMETHING=x`` is indistinguishable from
+ a second variable, which would let a command's *data* become the session's
+ *configuration*.
+ """
+ out: Dict[str, str] = {}
+ for record in text.split("0円"):
+ if record:
+ _split_env_record(record, out)
+ return out
+
+
+def _parse_env_lines(text: str) -> Dict[str, str]:
+ """Parse ``cmd.exe`` ``set`` output.
+
+ Windows environment values cannot contain a newline, so one line is exactly
+ one variable and an unparseable line is dropped rather than glued onto its
+ predecessor.
+ """
+ out: Dict[str, str] = {}
+ for raw in text.splitlines():
+ line = raw.rstrip("\r")
+ if not line or line.startswith("="):
+ # cmd.exe lists internal "=C:" / "=ExitCode" entries; not variables.
+ continue
+ _split_env_record(line, out)
+ return out
+
+
+def _terminate_tree(proc: "subprocess.Popen") -> None:
+ """Kill *proc* and everything it spawned.
+
+ Killing only the shell leaves the build it started running, which is what
+ makes a timed-out command worse than useless.
+ """
+ if proc.poll() is not None:
+ return
+ if os.name == "nt":
+ # No process groups: taskkill /T walks the parent-child tree instead.
+ # It takes seconds on a busy box, which is the price of not orphaning
+ # the build a timed-out command started.
+ try:
+ subprocess.run( # nosec B603 B607 - fixed argv, pid from our own child
+ ["taskkill", "/F", "/T", "/PID", str(proc.pid)],
+ capture_output=True,
+ check=False,
+ timeout=_TREE_KILL_TIMEOUT_SECONDS,
+ )
+ except subprocess.TimeoutExpired:
+ logger.warning("taskkill did not finish for pid %s", proc.pid)
+ proc.kill()
+ else:
+ try:
+ os.killpg(os.getpgid(proc.pid), signal.SIGKILL)
+ except (ProcessLookupError, PermissionError) as exc:
+ logger.warning("Could not kill process group for pid %s: %s", proc.pid, exc)
+ proc.kill()
+
+
+class ShellSession:
+ """A shell session whose cwd and exported environment persist across calls.
+
+ Calls on the same session are serialised by an internal lock, because they
+ share one logical shell state. Distinct sessions never collide: the cwd and
+ the variables are applied *inside the child*, so the calling process is
+ never mutated.
+ """
+
+ def __init__(
+ self,
+ start_cwd: Optional[str] = None,
+ shell: Optional[str] = None,
+ cwd_guard: Optional[Callable[[str], bool]] = None,
+ ):
+ """
+ Args:
+ start_cwd: Initial working directory. Defaults to the process cwd.
+ shell: Shell to run commands with. ``None`` means ``/bin/sh`` on
+ POSIX and ``cmd.exe`` on Windows; ``GAIA_SHELL`` overrides both.
+ Naming a POSIX shell on Windows makes the session generate a
+ shell script for it instead of a batch file.
+ cwd_guard: Consulted before absorbing a directory a command changed
+ into. Returning False keeps the session where it was — without
+ it, ``cd`` would be a way to reach paths the caller's path
+ policy refuses.
+ """
+ resolved = Path(start_cwd).resolve() if start_cwd else Path.cwd()
+ self._state = _State(cwd=str(resolved))
+ self._start_cwd = str(resolved)
+ self._baseline_env = {_env_key(k): v for k, v in os.environ.items()}
+ self._cwd_guard = cwd_guard
+ self._lock = threading.Lock()
+ self._closed = False
+ self._temp_dir: Optional[str] = None
+
+ configured = shell if shell is not None else os.environ.get("GAIA_SHELL", "")
+ configured = configured.strip()
+ if os.name == "nt":
+ self._shell = configured
+ self._posix_script = bool(configured)
+ else:
+ self._shell = configured or "/bin/sh"
+ self._posix_script = True
+
+ # -- state ------------------------------------------------------------
+
+ @property
+ def cwd(self) -> str:
+ """Current working directory of the session."""
+ return self._state.cwd
+
+ @property
+ def posix_script(self) -> bool:
+ """True when commands run as a POSIX shell script rather than a batch file."""
+ return self._posix_script
+
+ def environment(self) -> Dict[str, str]:
+ """Variables the session has diverged from the parent environment."""
+ return dict(self._state.overrides)
+
+ def removed_environment(self) -> List[str]:
+ """Inherited variables the session's commands have unset."""
+ return sorted(self._state.unset)
+
+ def effective_env(self) -> Dict[str, str]:
+ """The environment a command would see, for callers that bypass the script."""
+ env = os.environ.copy()
+ for name in self._state.unset:
+ env.pop(name, None)
+ env.update(self._state.overrides)
+ return env
+
+ def set_cwd(self, directory: str) -> bool:
+ """Set the working directory. False (and unchanged) if it is not a directory."""
+ resolved = Path(directory).resolve()
+ if not resolved.is_dir():
+ return False
+ self._state.cwd = str(resolved)
+ return True
+
+ def set_env(self, name: str, value: str) -> None:
+ """Set a variable for subsequent commands in this session."""
+ self._state.overrides[name] = value
+ self._state.unset.discard(name)
+
+ def reset(self) -> None:
+ """Forget every environment change and return to the starting directory."""
+ self._state = _State(cwd=self._start_cwd)
+
+ def close(self) -> None:
+ """Tear the session down. Later calls raise :class:`ShellSessionClosed`.
+
+ There is no resident child to kill — every command is waited on or
+ killed (with its process group) inside :meth:`run`. What is left is the
+ session's temp directory, which this removes.
+ """
+ self._closed = True
+ temp_dir, self._temp_dir = self._temp_dir, None
+ if temp_dir:
+ shutil.rmtree(temp_dir, ignore_errors=True)
+
+ @property
+ def closed(self) -> bool:
+ return self._closed
+
+ # -- execution --------------------------------------------------------
+
+ def run(
+ self,
+ command: str,
+ timeout: float = 30.0,
+ working_directory: Optional[str] = None,
+ ) -> ShellResult:
+ """Run *command* in the session, then absorb the state it left behind.
+
+ Args:
+ command: The shell command to execute.
+ timeout: Seconds before the command's process group is killed.
+ working_directory: Run this one command elsewhere. A one-shot
+ override: the session's own directory is untouched, and a
+ ``cd`` inside such a command is not absorbed.
+
+ Raises:
+ ShellSessionClosed: the session was torn down.
+ ShellSessionBusy: another command still holds the session.
+ """
+ if self._closed:
+ raise ShellSessionClosed(
+ "This shell session was closed. Start a new task, or reset the "
+ "session, to run commands again."
+ )
+ if not command.strip():
+ raise ValueError("Empty command")
+
+ # Bounded, so a command still running past its tool timeout (#2600)
+ # surfaces as an actionable error instead of wedging the agent.
+ if not self._lock.acquire(timeout=max(1.0, float(timeout))):
+ raise ShellSessionBusy(
+ "Another command is still running in this shell session. Wait for "
+ "it to finish, or reset the session to abandon it and start clean."
+ )
+ try:
+ return self._run_locked(command, timeout, working_directory)
+ finally:
+ self._lock.release()
+
+ def run_argv(
+ self,
+ argv: List[str],
+ timeout: float = 30.0,
+ working_directory: Optional[str] = None,
+ ) -> ShellResult:
+ """Run *argv* directly, with the session's cwd and environment applied.
+
+ For callers that must not hand a command string to a shell — a binary
+ invoked with arguments built from untrusted text. The session's state is
+ applied, but nothing is absorbed: an argv call cannot ``cd`` or
+ ``export`` in the first place.
+ """
+ if self._closed:
+ raise ShellSessionClosed(
+ "This shell session was closed. Start a new task, or reset the "
+ "session, to run commands again."
+ )
+ if not self._lock.acquire(timeout=max(1.0, float(timeout))):
+ raise ShellSessionBusy(
+ "Another command is still running in this shell session. Wait for "
+ "it to finish, or reset the session to abandon it and start clean."
+ )
+ try:
+ cwd = working_directory or self._state.cwd
+ result = self._spawn(argv, cwd, self.effective_env(), timeout, shell=False)
+ result.cwd = self._state.cwd
+ return result
+ finally:
+ self._lock.release()
+
+ # -- internals --------------------------------------------------------
+
+ def _run_locked(
+ self,
+ command: str,
+ timeout: float,
+ working_directory: Optional[str],
+ ) -> ShellResult:
+ temp_dir = self._ensure_temp_dir()
+ script_ext = ".sh" if self._posix_script else ".cmd"
+ start_cwd = working_directory or self._state.cwd
+
+ state_file = self._write_temp(temp_dir, ".state", "")
+ command_file = self._write_temp(temp_dir, script_ext, command + "\n")
+ script_file = self._write_temp(
+ temp_dir,
+ script_ext,
+ self._build_script(start_cwd, command_file, state_file),
+ )
+
+ try:
+ if self._posix_script:
+ argv = [self._shell, _generic_path(script_file)]
+ else:
+ # /d skips AutoRun, which would otherwise prepend a user's
+ # profile commands to every agent command.
+ argv = [os.environ.get("COMSPEC", "cmd.exe"), "/d", "/c", script_file]
+
+ # No cwd/env arguments: the script applies both inside the child, so
+ # the calling process is never mutated and sessions cannot collide.
+ result = self._spawn(argv, None, None, timeout, shell=False)
+ rejected = self._absorb_state(
+ self._read_state(state_file), absorb_cwd=working_directory is None
+ )
+ result.cwd = start_cwd if working_directory else self._state.cwd
+ result.cwd_change_rejected = rejected
+ return result
+ finally:
+ for path in (state_file, command_file, script_file):
+ try:
+ os.remove(path)
+ except OSError as exc:
+ logger.warning("Could not remove temp file %s: %s", path, exc)
+
+ def _ensure_temp_dir(self) -> str:
+ if self._temp_dir is None or not os.path.isdir(self._temp_dir):
+ self._temp_dir = tempfile.mkdtemp(prefix="gaia_shell_")
+ return self._temp_dir
+
+ @staticmethod
+ def _write_temp(temp_dir: str, extension: str, contents: str) -> str:
+ """Create a uniquely-named file in *temp_dir* and write *contents* to it.
+
+ Exclusive creation, so a pre-planted symlink cannot be written through.
+ """
+ fd, path = tempfile.mkstemp(suffix=extension, dir=temp_dir)
+ with os.fdopen(fd, "w", encoding="utf-8", newline="") as handle:
+ handle.write(contents)
+ return path
+
+ def _build_script(self, cwd: str, command_file: str, state_file: str) -> str:
+ """The per-command script: restore state, run the command, report state.
+
+ The command lives in its own file and is sourced (``.`` / ``call``)
+ rather than pasted in here. Pasting lets a command ending in a line
+ continuation, or an unterminated heredoc, swallow the framework's own
+ bookkeeping lines — which corrupts the reported exit code and leaks
+ internals into what the model reads back.
+ """
+ state = self._state
+ lines: List[str] = []
+ if self._posix_script:
+ lines.append(f"cd {_posix_quote(_generic_path(cwd))} &#124;&#124; exit 127")
+ for name in sorted(state.unset):
+ if _is_valid_env_name(name):
+ lines.append(f"unset {name}")
+ for name, value in sorted(state.overrides.items()):
+ if _is_valid_env_name(name):
+ lines.append(f"{name}={_posix_quote(value)}; export {name}")
+ lines.append(f". {_posix_quote(_generic_path(command_file))} < /dev/null") + lines.append("__gaia_rc=$?") + # awk's ENVIRON gives NUL-delimited records; `env` output cannot be + # parsed unambiguously (see _parse_env_records_nul). + lines.append( + "{ pwd; printf '%s\\n' " + + _posix_quote(_ENV_MARKER) + + '; awk \'BEGIN { for (k in ENVIRON) printf "%s=%s%c", k, ' + "ENVIRON[k], 0 }'; }> "
+ + _posix_quote(_generic_path(state_file))
+ + " 2>/dev/null"
+ )
+ lines.append("exit $__gaia_rc")
+ return "\n".join(lines) + "\n"
+
+ lines.append("@echo off")
+ lines.append(f'cd /d "{_batch_quote(cwd)}"')
+ lines.append("if errorlevel 1 exit /b 127")
+ for name in sorted(state.unset):
+ if _is_valid_env_name(name):
+ lines.append(f'set "{name}="')
+ for name, value in sorted(state.overrides.items()):
+ if _is_valid_env_name(name):
+ lines.append(f'set "{name}={_batch_quote(value)}"')
+ lines.append(f'call "{command_file}" <nul') + lines.append("set __GAIA_RC=%ERRORLEVEL%") + lines.append(f'> "{state_file}" (')
+ lines.append(" cd")
+ lines.append(f" echo {_ENV_MARKER}")
+ lines.append(" set")
+ lines.append(")")
+ lines.append("exit /b %__GAIA_RC%")
+ return "\r\n".join(lines) + "\r\n"
+
+ def _read_state(self, state_file: str) -> str:
+ try:
+ raw = Path(state_file).read_bytes()
+ except OSError:
+ return ""
+ encoding = "utf-8" if self._posix_script else locale.getpreferredencoding(False)
+ return raw.decode(encoding, errors="replace")
+
+ def _absorb_state(self, state_text: str, absorb_cwd: bool) -> Optional[str]:
+ """Take on the cwd and environment the command left behind.
+
+ A command that calls ``exit`` terminates the script before the
+ bookkeeping runs, so its changes are not captured; the session keeps its
+ previous state rather than guessing.
+
+ Returns a message when a directory change was refused by the cwd guard.
+ """
+ marker = state_text.find(_ENV_MARKER)
+ if marker == -1:
+ return None
+
+ rejected: Optional[str] = None
+ reported_cwd = state_text[:marker].strip()
+ if reported_cwd and absorb_cwd:
+ # The shell is the authority on where it ended up. With a Git Bash
+ # shell on Windows that is an MSYS path (`/c/...`) the Win32 API
+ # does not recognise but the next script — run by the same shell —
+ # does, so it is taken as reported rather than validated away.
+ if self._cwd_guard is not None and not self._cwd_guard(reported_cwd):
+ rejected = (
+ f"Directory change to '{reported_cwd}' was not applied: that "
+ "path is outside the directories this agent may access. The "
+ f"session stayed in '{self._state.cwd}'."
+ )
+ logger.info("Refused session cwd change to %r", reported_cwd)
+ else:
+ self._state.cwd = reported_cwd
+
+ env_text = state_text[marker + len(_ENV_MARKER) :].lstrip("\r\n")
+ captured = (
+ _parse_env_records_nul(env_text)
+ if self._posix_script
+ else _parse_env_lines(env_text)
+ )
+ if not captured:
+ return rejected
+
+ overrides: Dict[str, str] = {}
+ unset = set()
+ # Only replayable names participate: `ProgramFiles(x86)` is a real
+ # Windows variable but no `set NAME=` / `export NAME` can name it.
+ captured_keys = {
+ _env_key(name) for name in captured if _is_valid_env_name(name)
+ }
+ for name, value in captured.items():
+ key = _env_key(name)
+ if key in _VOLATILE_ENV_NAMES or not _is_valid_env_name(name):
+ continue
+ if self._baseline_env.get(key) != value:
+ overrides[name] = value
+ for key in self._baseline_env:
+ if key in _VOLATILE_ENV_NAMES or not _is_valid_env_name(key):
+ continue
+ if key not in captured_keys:
+ unset.add(key)
+ self._state.overrides = overrides
+ self._state.unset = unset
+ return rejected
+
+ @staticmethod
+ def _spawn(
+ argv,
+ cwd: Optional[str],
+ env: Optional[Dict[str, str]],
+ timeout: float,
+ shell: bool,
+ ) -> ShellResult:
+ """Run *argv*, capping the wait and killing the whole tree on expiry.
+
+ encoding/errors are explicit, and load-bearing. Bare ``text=True``
+ decodes with the locale codec — cp1252 on a default Windows box — inside
+ subprocess's pipe reader THREAD. A byte that codec cannot map raises
+ there, the thread dies, and the caller gets returncode 0 with EMPTY
+ stdout: the command succeeded and its output was silently discarded.
+ """
+ start = time.monotonic()
+ popen_kwargs = {
+ "stdout": subprocess.PIPE,
+ "stderr": subprocess.PIPE,
+ # stdin is DEVNULL, never inherited. This process's stdin is the
+ # agent transport's pipe — held open and never written to — so a
+ # child that reads it blocks forever on input that cannot arrive.
+ "stdin": subprocess.DEVNULL,
+ "cwd": cwd,
+ "env": env,
+ "encoding": "utf-8",
+ "errors": "replace",
+ "shell": shell,
+ }
+ if os.name != "nt":
+ # Its own process group, so a timeout can kill the command's
+ # children too rather than just the shell that started them.
+ popen_kwargs["start_new_session"] = True
+
+ with subprocess.Popen(argv, **popen_kwargs) as proc: # nosec B603
+ try:
+ stdout, stderr = proc.communicate(timeout=timeout)
+ timed_out = False
+ except subprocess.TimeoutExpired:
+ _terminate_tree(proc)
+ timed_out = True
+ try:
+ stdout, stderr = proc.communicate(timeout=_DRAIN_TIMEOUT_SECONDS)
+ except subprocess.TimeoutExpired:
+ stdout, stderr = "", ""
+ return_code = proc.returncode if proc.returncode is not None else -1
+
+ return ShellResult(
+ stdout=stdout or "",
+ stderr=stderr or "",
+ return_code=return_code,
+ timed_out=timed_out,
+ duration_seconds=time.monotonic() - start,
+ )
diff --git a/tests/unit/test_shell_session.py b/tests/unit/test_shell_session.py
new file mode 100644
index 000000000..ec6bebe8d
--- /dev/null
+++ b/tests/unit/test_shell_session.py
@@ -0,0 +1,253 @@
+# Copyright(C) 2025-2026 Advanced Micro Devices, Inc. All rights reserved.
+# SPDX-License-Identifier: MIT
+"""Tests for the persistent shell session (issue #3380).
+
+These run real subprocesses on purpose. A mocked session would prove only that
+we called ``subprocess`` — it could not prove that ``cd`` in one call is
+visible to the next, which is the entire feature.
+"""
+
+import os
+import sys
+import threading
+import time
+
+import pytest
+
+from gaia.agents.tools.shell_session import (
+ ShellSession,
+ ShellSessionBusy,
+ ShellSessionClosed,
+)
+
+IS_WINDOWS = os.name == "nt"
+
+
+def export_command(name: str, value: str) -> str:
+ """The platform's way of exporting a variable for later commands."""
+ return f"set {name}={value}" if IS_WINDOWS else f"export {name}={value}"
+
+
+def echo_var_command(name: str) -> str:
+ return f"echo %{name}%" if IS_WINDOWS else f'echo "${name}"'
+
+
+def sleep_command(seconds: float, marker: str = "") -> str:
+ """A command whose *child process* blocks, then optionally leaves a marker.
+
+ A grandchild rather than the shell itself: killing only the shell is the
+ failure mode the process-group kill exists to prevent, and a marker written
+ after the sleep is how a survivor announces itself.
+ """
+ body = f"import time; time.sleep({seconds})"
+ if marker:
+ body += f"; open(r'{marker}', 'w').write('survived')"
+ return f'"{sys.executable}" -c "{body}"'
+
+
+@pytest.fixture
+def session(tmp_path):
+ shell = ShellSession(start_cwd=str(tmp_path))
+ yield shell
+ shell.close()
+
+
+class TestWorkingDirectoryPersists:
+ def test_cd_survives_to_the_next_command(self, session, tmp_path):
+ (tmp_path / "sub").mkdir()
+
+ session.run("cd sub")
+
+ assert session.cwd.replace("\\", "/").endswith("/sub")
+ result = session.run("cd" if IS_WINDOWS else "pwd")
+ assert "sub" in result.stdout
+
+ def test_working_directory_argument_is_one_shot(self, session, tmp_path):
+ """An explicit per-call directory scopes that call, as it always has."""
+ (tmp_path / "elsewhere").mkdir()
+ before = session.cwd
+
+ session.run(
+ "cd" if IS_WINDOWS else "pwd", working_directory=str(tmp_path / "elsewhere")
+ )
+
+ assert session.cwd == before
+
+ def test_set_cwd_rejects_a_non_directory(self, session, tmp_path):
+ before = session.cwd
+
+ assert session.set_cwd(str(tmp_path / "does-not-exist")) is False
+ assert session.cwd == before
+
+ def test_cwd_guard_refuses_a_directory_change(self, tmp_path):
+ (tmp_path / "off-limits").mkdir()
+ allowed = str(tmp_path)
+ shell = ShellSession(
+ start_cwd=allowed,
+ cwd_guard=lambda path: "off-limits" not in path.replace("\\", "/"),
+ )
+ try:
+ result = shell.run("cd off-limits")
+
+ assert shell.cwd == allowed
+ assert result.cwd_change_rejected is not None
+ finally:
+ shell.close()
+
+
+class TestEnvironmentPersists:
+ def test_exported_variable_survives_to_the_next_command(self, session):
+ session.run(export_command("GAIA_TEST_VAR", "persisted"))
+
+ assert session.environment().get("GAIA_TEST_VAR") == "persisted"
+ result = session.run(echo_var_command("GAIA_TEST_VAR"))
+ assert "persisted" in result.stdout
+
+ def test_inherited_variables_are_not_reported_as_changes(self, session):
+ """Only what the session actually diverged, or Windows folds every name."""
+ session.run("echo hello")
+
+ # pytest rewrites PYTEST_CURRENT_TEST after the session took its
+ # baseline, so the session is right to call it changed.
+ diverged = session.environment()
+ diverged.pop("PYTEST_CURRENT_TEST", None)
+ assert diverged == {}
+ assert session.removed_environment() == []
+
+ def test_set_env_applies_to_the_next_command(self, session):
+ session.set_env("GAIA_TEST_PRESET", "from-api")
+
+ result = session.run(echo_var_command("GAIA_TEST_PRESET"))
+ assert "from-api" in result.stdout
+
+ def test_a_virtualenv_style_activation_persists(self, session, tmp_path):
+ """What venv activation actually is: PATH plus a marker variable."""
+ session.run(export_command("VIRTUAL_ENV", str(tmp_path / "venv")))
+
+ assert "VIRTUAL_ENV" in session.environment()
+ result = session.run(echo_var_command("VIRTUAL_ENV"))
+ assert "venv" in result.stdout
+
+ def test_the_parent_process_is_never_mutated(self, session, tmp_path):
+ before_cwd = os.getcwd()
+ session.run(export_command("GAIA_TEST_LEAK", "leaked"))
+ session.run("cd .")
+
+ assert "GAIA_TEST_LEAK" not in os.environ
+ assert os.getcwd() == before_cwd
+
+
+class TestReset:
+ def test_reset_restores_the_starting_state(self, session, tmp_path):
+ (tmp_path / "sub").mkdir()
+ session.run("cd sub")
+ session.run(export_command("GAIA_TEST_RESET", "x"))
+
+ session.reset()
+
+ assert session.cwd == str(tmp_path.resolve())
+ assert session.environment() == {}
+ result = session.run(echo_var_command("GAIA_TEST_RESET"))
+ assert result.stdout.strip() != "x"
+
+
+class TestTeardown:
+ def test_close_makes_further_commands_an_error(self, tmp_path):
+ shell = ShellSession(start_cwd=str(tmp_path))
+ shell.run("echo hello")
+
+ shell.close()
+
+ assert shell.closed is True
+ with pytest.raises(ShellSessionClosed):
+ shell.run("echo hello")
+
+ def test_close_removes_the_session_temp_directory(self, tmp_path):
+ shell = ShellSession(start_cwd=str(tmp_path))
+ shell.run("echo hello")
+ temp_dir = shell._temp_dir # pylint: disable=protected-access
+ assert temp_dir and os.path.isdir(temp_dir)
+
+ shell.close()
+
+ assert not os.path.isdir(temp_dir)
+
+ def test_a_timed_out_command_leaves_no_running_child(self, session, tmp_path):
+ """The command's children are killed too, not just the shell."""
+ marker = tmp_path / "survivor.txt"
+
+ result = session.run(sleep_command(4, str(marker)), timeout=1)
+
+ assert result.timed_out is True
+ time.sleep(6)
+ assert not marker.exists(), "a child outlived the timeout and kept running"
+
+
+class TestSerialisation:
+ def test_a_second_command_waits_for_the_first(self, session):
+ """Two threads against one session must not interleave."""
+ order = []
+ barrier = threading.Barrier(2)
+
+ def slow():
+ barrier.wait()
+ session.run(sleep_command(2), timeout=20)
+ order.append("slow")
+
+ def quick():
+ barrier.wait()
+ time.sleep(0.3)
+ session.run("echo quick", timeout=20)
+ order.append("quick")
+
+ threads = [threading.Thread(target=slow), threading.Thread(target=quick)]
+ for thread in threads:
+ thread.start()
+ for thread in threads:
+ thread.join(timeout=60)
+
+ assert order == ["slow", "quick"]
+
+ def test_a_busy_session_says_so_instead_of_hanging(self, session):
+ """#2600: a command past its tool timeout is still inside the session."""
+ started = threading.Event()
+
+ def hog():
+ started.set()
+ session.run(sleep_command(6), timeout=30)
+
+ thread = threading.Thread(target=hog)
+ thread.start()
+ started.wait(timeout=5)
+ time.sleep(0.5)
+ try:
+ with pytest.raises(ShellSessionBusy):
+ session.run("echo blocked", timeout=1)
+ finally:
+ thread.join(timeout=60)
+
+
+class TestExecution:
+ def test_exit_code_is_reported(self, session):
+ result = session.run("exit 3")
+
+ assert result.return_code == 3
+
+ def test_stdout_is_captured(self, session):
+ result = session.run("echo captured")
+
+ assert "captured" in result.stdout
+ assert result.return_code == 0
+
+ def test_run_argv_applies_the_session_environment(self, session):
+ session.set_env("GAIA_TEST_ARGV", "argv-value")
+
+ result = session.run_argv(
+ [sys.executable, "-c", "import os; print(os.environ['GAIA_TEST_ARGV'])"]
+ )
+
+ assert "argv-value" in result.stdout
+
+ def test_empty_command_is_refused(self, session):
+ with pytest.raises(ValueError):
+ session.run(" ")
From 7dce8587cd253108f5ef20f10163f413e115eeb0 Mon Sep 17 00:00:00 2001
From: Ovtcharov <kovtchar@amd.com>
Date: Sat, 5 Sep 2026 03:51:44 -0700
Subject: [PATCH 2/4] feat(shell): run_shell_command in a persistent session,
 with a probe and a reset
A directory change or a variable set in one call is now still in effect for the
next. Adds get_shell_state so the agent can read where it is rather than infer
it, set_shell_variable because the shell's own export is not on the read-only
whitelist, and reset_shell_session to recover a session that is wedged or in the
wrong place.
Command validation is unchanged and still applies per segment. Two seams needed
care: a skill-granted binary still runs as argv, never as a string handed to a
shell, and a cd into a directory the path policy refuses is not absorbed -
otherwise persistence would be a way to read files by bare name from anywhere.
---
 .../python/gaia_agent_chat/tool_bundles.py &#124; 15 +-
 src/gaia/agents/tools/shell_session.py &#124; 19 +
 src/gaia/agents/tools/shell_tools.py &#124; 368 ++++++++++++------
 tests/unit/test_shell_output_encoding.py &#124; 34 +-
 tests/unit/test_shell_tools_session.py &#124; 164 ++++++++
 5 files changed, 456 insertions(+), 144 deletions(-)
 create mode 100644 tests/unit/test_shell_tools_session.py
diff --git a/hub/agents/chat/python/gaia_agent_chat/tool_bundles.py b/hub/agents/chat/python/gaia_agent_chat/tool_bundles.py
index 00b396dda..80d7b698c 100644
--- a/hub/agents/chat/python/gaia_agent_chat/tool_bundles.py
+++ b/hub/agents/chat/python/gaia_agent_chat/tool_bundles.py
@@ -124,8 +124,16 @@
 ),
 ToolBundle(
 name="shell",
- members=frozenset({"run_shell_command", "get_system_info"}),
- description="Run shell commands and query the system.",
+ members=frozenset(
+ {
+ "run_shell_command",
+ "get_shell_state",
+ "set_shell_variable",
+ "reset_shell_session",
+ "get_system_info",
+ }
+ ),
+ description="Run shell commands in a persistent session and query the system.",
 ),
 ToolBundle(
 name="clipboard",
@@ -351,6 +359,9 @@
 members=frozenset(
 {
 "run_shell_command",
+ "get_shell_state",
+ "set_shell_variable",
+ "reset_shell_session",
 "execute_python_file",
 "get_system_info",
 }
diff --git a/src/gaia/agents/tools/shell_session.py b/src/gaia/agents/tools/shell_session.py
index e6b47b8d3..d7a0753f4 100644
--- a/src/gaia/agents/tools/shell_session.py
+++ b/src/gaia/agents/tools/shell_session.py
@@ -336,8 +336,24 @@ def close(self) -> None:
 There is no resident child to kill — every command is waited on or
 killed (with its process group) inside :meth:`run`. What is left is the
 session's temp directory, which this removes.
+
+ A command still running past its tool timeout (#2600) is holding files
+ in that directory, so the removal is handed to it rather than pulled out
+ from under it.
 """
 self._closed = True
+ if self._lock.acquire(blocking=False):
+ try:
+ self._discard_temp_dir()
+ finally:
+ self._lock.release()
+ else:
+ logger.info(
+ "Shell session closed while a command was still running; its temp "
+ "directory is removed when that command finishes."
+ )
+
+ def _discard_temp_dir(self) -> None:
 temp_dir, self._temp_dir = self._temp_dir, None
 if temp_dir:
 shutil.rmtree(temp_dir, ignore_errors=True)
@@ -461,6 +477,9 @@ def _run_locked(
 os.remove(path)
 except OSError as exc:
 logger.warning("Could not remove temp file %s: %s", path, exc)
+ if self._closed:
+ # Closed while this command was running; it is ours to clean up.
+ self._discard_temp_dir()
 
 def _ensure_temp_dir(self) -> str:
 if self._temp_dir is None or not os.path.isdir(self._temp_dir):
diff --git a/src/gaia/agents/tools/shell_tools.py b/src/gaia/agents/tools/shell_tools.py
index d4fabe917..e46ef7dd8 100644
--- a/src/gaia/agents/tools/shell_tools.py
+++ b/src/gaia/agents/tools/shell_tools.py
@@ -10,11 +10,17 @@
 import os
 import re
 import shlex
-import subprocess
+import shutil
 import time
 from collections import deque
 from pathlib import Path
-from typing import Any, Dict, Optional
+from typing import Any, Callable, Dict, Optional
+
+from gaia.agents.tools.shell_session import (
+ ShellSession,
+ ShellSessionBusy,
+ ShellSessionClosed,
+)
 
 logger = logging.getLogger(__name__)
 
@@ -198,6 +204,33 @@
 #: only one a ``shell:execute`` grant may exempt from confirmation.
 _POLICY_GATED_SHELL_TOOL = "run_shell_command"
 
+#: Variables that decide which binary or which code a later command actually
+#: runs. Setting one turns the read-only whitelist into a name the agent
+#: controls: `PATH` picked the `ls` that ran, `PYTHONPATH` picked the module a
+#: script imported. The whitelist checks names, not what they resolve to, so
+#: these stay with the parent process.
+_UNSETTABLE_ENV_PREFIXES = ("LD_", "DYLD_")
+_UNSETTABLE_ENV_NAMES = frozenset(
+ {
+ "PATH",
+ "PATHEXT",
+ "COMSPEC",
+ "SHELL",
+ "IFS",
+ "ENV",
+ "BASH_ENV",
+ "PYTHONPATH",
+ "PYTHONHOME",
+ "PYTHONSTARTUP",
+ "NODE_OPTIONS",
+ "PERL5LIB",
+ "RUBYOPT",
+ "GAIA_SHELL",
+ }
+)
+
+_VALID_SHELL_VAR_NAME = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$")
+
 
 def skill_granted_binaries(host: Any) -> frozenset:
 """CLIs *host*'s loaded skills granted via ``shell:execute:<binary>``.
@@ -268,6 +301,12 @@ class ShellToolsMixin:
 
 Tools provided:
 - run_shell_command: Execute terminal commands with timeout and safety checks
+ - get_shell_state: Read the session's directory and changed environment
+ - reset_shell_session: Return the session to the state it started in
+
+ Commands share one :class:`ShellSession` per agent, so a directory change or
+ an exported variable is still there on the next call. Access is serialised;
+ validation is unchanged and still applies per command.
 
 Rate Limiting:
 - Max 10 commands per minute to prevent DOS
@@ -283,6 +322,55 @@ def __init__(self, *args, **kwargs):
 self.max_commands_per_minute = 10
 self.max_commands_per_10_seconds = 3
 
+ # Created on first use: an agent that never runs a command should not
+ # pay for a temp directory.
+ self._shell_session: Optional[ShellSession] = None
+
+ def _session_cwd_guard(self) -> Optional[Callable[[str], bool]]:
+ """The predicate the session asks before following a ``cd``.
+
+ Without it, persistence would be a way around the path policy: ``cd`` to
+ a forbidden directory, then read a file by bare name, and the per-argument
+ check never sees a path to reject.
+ """
+ validator = getattr(self, "path_validator", None)
+ if validator is not None:
+ return validator.is_path_allowed
+ checker = getattr(self, "_is_path_allowed", None)
+ if checker is not None:
+ return checker
+ return None
+
+ @property
+ def shell_session(self) -> ShellSession:
+ """This agent's shell session, created on first use."""
+ session = getattr(self, "_shell_session", None)
+ if session is None or session.closed:
+ session = ShellSession(cwd_guard=self._session_cwd_guard())
+ self._shell_session = session
+ return session
+
+ def reset_shell_session(self) -> ShellSession:
+ """Replace the session with a clean one and return it.
+
+ Replaced rather than reset in place, so this also recovers a session
+ wedged by a command that outlived its tool timeout (#2600): the new
+ session has its own lock, and the old one is closed as soon as the
+ straggler lets go of it.
+ """
+ previous = getattr(self, "_shell_session", None)
+ self._shell_session = ShellSession(cwd_guard=self._session_cwd_guard())
+ if previous is not None:
+ previous.close()
+ return self._shell_session
+
+ def close_shell_session(self) -> None:
+ """Tear the session down at task end. Safe to call more than once."""
+ session = getattr(self, "_shell_session", None)
+ if session is not None:
+ session.close()
+ self._shell_session = None
+
 def _validate_shell_command(self, command: str) -> tuple:
 """Every refusal ``command`` earns on its text alone, plus its segments.
 
@@ -829,9 +917,14 @@ def run_shell_command(
 "has_errors": True,
 }
 
- cwd = str(Path(working_directory).resolve())
+ session_cwd_override = str(Path(working_directory).resolve())
+ cwd = session_cwd_override
 else:
- cwd = str(Path.cwd())
+ # The session's directory, not the process's: a `cd` from an
+ # earlier call is where this command actually runs, so it is
+ # also what relative arguments must be checked against.
+ session_cwd_override = None
+ cwd = self.shell_session.cwd
 
 # Operators, syntax, and the per-command whitelist. Shared with
 # the pre-flight that runs before the confirmation prompt, so a
@@ -922,22 +1015,16 @@ def run_shell_command(
 if hasattr(self, "debug") and self.debug:
 logger.info(f"Executing command: {command} in {cwd}")
 
- # On Windows, many commands are shell built-ins (dir, cd, type,
- # echo) and Unix commands (ls, pwd, cat) don't exist as .exe
- # files. Since we have already validated the command against the
- # whitelist, we use shell=True on Windows so cmd.exe can resolve
- # both built-ins and commands on PATH (including those from Git
- # for Windows which provides ls, cat, grep, etc.).
- #
- # A skill-granted CLI is the exception, and must stay one. It is
- # a real executable — it needs no built-in resolution — and it
- # is the one path that can run without a confirmation prompt, on
- # arguments built from untrusted remote text (an issue body the
- # model just read). Handing cmd.exe the raw STRING there would
- # let that text act: `--search "x&#124;whoami"` is one argv token to
- # every check above and two commands to cmd.exe, and `%VAR%`
- # expands into a value the approval prompt never showed. argv
- # goes to the process verbatim, so neither is possible.
+ # A skill-granted CLI runs as argv, never as a command string,
+ # and must keep doing so. It is a real executable — it needs no
+ # built-in resolution — and it is the one path that can run
+ # without a confirmation prompt, on arguments built from
+ # untrusted remote text (an issue body the model just read).
+ # Handing a shell the raw STRING there would let that text act:
+ # `--search "x&#124;whoami"` is one argv token to every check above
+ # and two commands to a shell, and `%VAR%` expands into a value
+ # the approval prompt never showed. argv goes to the process
+ # verbatim, so neither is possible.
 # One segment only: a pipeline needs a shell to be a pipeline,
 # and `cmd_parts` has already dropped the `&#124;` tokens, so an argv
 # run of one would silently concatenate the two commands.
@@ -946,19 +1033,12 @@ def run_shell_command(
 and bool(granted)
 and _is_granted_binary(segments[0][0], granted)
 )
- use_shell = os.name == "nt" and not lone_granted_segment
-
- # Build the command string for execution
- # On Windows with shell=True, use the ORIGINAL command string
- # to preserve quoting (critical for PowerShell pipe commands)
- exec_cmd = cmd_parts # Default: list for subprocess
 
- if use_shell:
- # Start with original command to preserve quoting
- exec_cmd = command
-
- # Map common Unix commands to Windows equivalents
- # when Git-for-Windows tools aren't on PATH
+ session = self.shell_session
+ exec_command = command
+ if not lone_granted_segment and not session.posix_script:
+ # cmd.exe: map Unix names to their built-ins when the
+ # Git-for-Windows tools that provide them aren't on PATH.
 _UNIX_TO_WIN = {
 "ls": "dir",
 "pwd": "cd",
@@ -967,106 +1047,62 @@ def run_shell_command(
 "cp": "copy",
 "mv": "move",
 }
- if cmd_base in _UNIX_TO_WIN:
- import shutil
-
- if not shutil.which(cmd_base):
- win_cmd = _UNIX_TO_WIN[cmd_base]
- logger.info(
- f"Mapping Unix command '{cmd_base}' -> Windows '{win_cmd}'"
- )
- # Replace just the command name in the original string
- exec_cmd = win_cmd + exec_cmd[len(cmd_base) :]
-
- # Execute command
- #
- # encoding/errors are explicit, and load-bearing. Bare
- # ``text=True`` decodes with the locale codec — cp1252 on a
- # default Windows box — and subprocess does that decode inside
- # its pipe reader THREAD. A byte that codec cannot map raises
- # UnicodeDecodeError in that thread, which dies, and
- # subprocess.run then returns returncode 0 with EMPTY stdout.
- # The command succeeded and its output was silently discarded.
- #
- # That is not an edge case: `gh issue list` on amd/gaia returns
- # an issue title containing "⚠️", so GitHub triage got back
- # nothing and the model reported an empty backlog it had never
- # actually read. Any tool emitting UTF-8 (git, gh, npm, docker)
- # hits it. errors="replace" keeps a stray undecodable byte from
- # costing the whole output.
- start_time = time.monotonic()
+ if cmd_base in _UNIX_TO_WIN and not shutil.which(cmd_base):
+ win_cmd = _UNIX_TO_WIN[cmd_base]
+ logger.info(
+ f"Mapping Unix command '{cmd_base}' -> Windows '{win_cmd}'"
+ )
+ exec_command = win_cmd + command[len(cmd_base) :]
+
 try:
- result = subprocess.run(
- exec_cmd,
- cwd=cwd,
- capture_output=True,
- # stdin is DEVNULL, never inherited. capture_output
- # redirects stdout/stderr but leaves stdin alone, and
- # this process's stdin is the agent transport's pipe —
- # held open by the TUI and never written to. A child
- # that reads it (directly, or by probing whether it is
- # interactive) blocks forever on input that cannot
- # arrive, because there is no human on that pipe.
- #
- # The hang was not theoretical: `gh` spawned from the
- # agent never exited, while the identical command took
- # 0.07s from a shell. Worse, subprocess.run's own
- # timeout does not save it — on expiry it kills the
- # cmd.exe it launched, then calls communicate() again
- # with NO timeout, which waits on pipes the surviving
- # grandchild still holds. That is the 180s tool timeout
- # and the orphaned gh.exe left behind by every attempt.
- #
- # DEVNULL gives an immediate EOF, which is the honest
- # answer here: an agent's shell command is
- # non-interactive by construction.
- stdin=subprocess.DEVNULL,
- encoding="utf-8",
- errors="replace",
- timeout=timeout,
- check=False,
- env=os.environ.copy(),
- shell=use_shell, # nosec B602 - Windows-only; command whitelist-validated above, shell needed for cmd.exe built-ins/pipes
- )
- duration = time.monotonic() - start_time
-
- # Record successful command execution for rate limiting
- self._record_command_execution()
- except subprocess.TimeoutExpired as exc:
- duration = time.monotonic() - start_time
-
- # Handle timeout gracefully
- stdout_str = ""
- stderr_str = ""
- if exc.stdout:
- stdout_str = (
- exc.stdout
- if isinstance(exc.stdout, str)
- else exc.stdout.decode("utf-8", errors="replace")
+ if lone_granted_segment:
+ result = session.run_argv(
+ cmd_parts,
+ timeout=timeout,
+ working_directory=session_cwd_override,
 )
- if exc.stderr:
- stderr_str = (
- exc.stderr
- if isinstance(exc.stderr, str)
- else exc.stderr.decode("utf-8", errors="replace")
+ else:
+ result = session.run(
+ exec_command,
+ timeout=timeout,
+ working_directory=session_cwd_override,
 )
+ except ShellSessionBusy as exc:
+ return {
+ "status": "error",
+ "error": str(exc),
+ "command": command,
+ "has_errors": True,
+ "session_busy": True,
+ "hint": "Call reset_shell_session to abandon the stuck command and start clean.",
+ }
+ except ShellSessionClosed as exc:
+ return {
+ "status": "error",
+ "error": str(exc),
+ "command": command,
+ "has_errors": True,
+ }
 
+ if result.timed_out:
 return {
 "status": "error",
 "error": f"Command timed out after {timeout} seconds",
 "command": command,
- "stdout": stdout_str,
- "stderr": stderr_str,
+ "stdout": result.stdout,
+ "stderr": result.stderr,
 "has_errors": True,
 "timed_out": True,
 "timeout": timeout,
- "duration_seconds": duration,
- "cwd": cwd,
+ "duration_seconds": result.duration_seconds,
+ "cwd": result.cwd,
 }
 
+ self._record_command_execution()
+
 # Capture and truncate output if too long
- stdout = result.stdout or ""
- stderr = result.stderr or ""
+ stdout = result.stdout
+ stderr = result.stderr
 truncated = False
 max_output = 10_000
 
@@ -1081,22 +1117,106 @@ def run_shell_command(
 # Debug logging
 if hasattr(self, "debug") and self.debug:
 logger.info(
- f"Command completed in {duration:.2f}s with return code {result.returncode}"
+ f"Command completed in {result.duration_seconds:.2f}s "
+ f"with return code {result.return_code}"
 )
 
- return {
+ response = {
 "status": "success",
 "command": command,
 "stdout": stdout,
 "stderr": stderr,
- "return_code": result.returncode,
- "has_errors": result.returncode != 0,
- "duration_seconds": duration,
+ "return_code": result.return_code,
+ "has_errors": result.return_code != 0,
+ "duration_seconds": result.duration_seconds,
 "timeout": timeout,
- "cwd": cwd,
+ "cwd": result.cwd,
+ "session_cwd": session.cwd,
 "output_truncated": truncated,
 }
+ if result.cwd_change_rejected:
+ response["warning"] = result.cwd_change_rejected
+ return response
 
 except Exception as exc:
 logger.error(f"Error executing shell command: {exc}")
 return {"status": "error", "error": str(exc), "has_errors": True}
+
+ @tool(
+ atomic=True,
+ display_label="Shell state",
+ )
+ def get_shell_state() -> Dict[str, Any]:
+ """Report the shell session's current directory and changed environment.
+
+ Shell commands share one session, so a `cd` or an exported variable
+ from an earlier command is still in effect. Call this to read that
+ state instead of guessing it.
+ """
+ session = self.shell_session
+ return {
+ "status": "success",
+ "cwd": session.cwd,
+ "environment": session.environment(),
+ "removed_environment": session.removed_environment(),
+ "interpreter": "posix-shell" if session.posix_script else "cmd.exe",
+ "has_errors": False,
+ }
+
+ @tool(
+ atomic=True,
+ display_label="Set shell variable",
+ )
+ def set_shell_variable(name: str, value: str) -> Dict[str, Any]:
+ """Set an environment variable for every later command this turn.
+
+ The shell's own `export` is not on the read-only command whitelist,
+ so this is how a variable gets set. Variables that decide which
+ binary or which code runs next — PATH, PYTHONPATH, LD_* and the
+ like — are refused.
+ """
+ upper = name.strip().upper()
+ if not _VALID_SHELL_VAR_NAME.match(name.strip()):
+ return {
+ "status": "error",
+ "error": f"'{name}' is not a valid environment variable name.",
+ "has_errors": True,
+ "hint": "Use letters, digits and underscores, starting with a letter or underscore.",
+ }
+ if upper in _UNSETTABLE_ENV_NAMES or upper.startswith(
+ _UNSETTABLE_ENV_PREFIXES
+ ):
+ return {
+ "status": "error",
+ "error": (
+ f"'{name}' decides which binary or which code a later command "
+ "runs, so it cannot be set from here."
+ ),
+ "has_errors": True,
+ "hint": "Pass the value on the command line instead, or use an absolute path.",
+ }
+ self.shell_session.set_env(name.strip(), value)
+ return {
+ "status": "success",
+ "message": f"{name.strip()} is set for the rest of this session.",
+ "has_errors": False,
+ }
+
+ @tool(
+ atomic=True,
+ display_label="Reset shell",
+ )
+ def reset_shell_session() -> Dict[str, Any]:
+ """Return the shell session to the directory and environment it started in.
+
+ Use this when the session is in a bad state — a wrong directory, a
+ variable that should not be set, or a command that hung and left the
+ session busy.
+ """
+ session = self.reset_shell_session()
+ return {
+ "status": "success",
+ "message": "Shell session reset.",
+ "cwd": session.cwd,
+ "has_errors": False,
+ }
diff --git a/tests/unit/test_shell_output_encoding.py b/tests/unit/test_shell_output_encoding.py
index 4990aee1d..b87a738bb 100644
--- a/tests/unit/test_shell_output_encoding.py
+++ b/tests/unit/test_shell_output_encoding.py
@@ -76,6 +76,20 @@ def test_output_with_non_locale_bytes_survives(tmp_path):
 assert "café" in result["stdout"]
 
 
+def _spawn_kwargs(monkeypatch) -> dict:
+ """The kwargs the executor hands the child process for `echo hello`."""
+ seen = {}
+ real_popen = subprocess.Popen
+
+ def spy(*args, **kwargs):
+ seen.update(kwargs)
+ return real_popen(*args, **kwargs)
+
+ monkeypatch.setattr(subprocess, "Popen", spy)
+ _shell_tool()(command="echo hello")
+ return seen
+
+
 def test_executor_decodes_utf8_not_the_locale(monkeypatch):
 """Pin the call itself, so a future edit cannot quietly restore text=True.
 
@@ -83,15 +97,7 @@ def test_executor_decodes_utf8_not_the_locale(monkeypatch):
 invisible in the result: the regression looks like a command that returned
 nothing, on every platform whose locale codec happens to be strict.
 """
- seen = {}
- real_run = subprocess.run
-
- def spy(*args, **kwargs):
- seen.update(kwargs)
- return real_run(*args, **kwargs)
-
- monkeypatch.setattr(subprocess, "run", spy)
- _shell_tool()(command="echo hello")
+ seen = _spawn_kwargs(monkeypatch)
 
 assert seen.get("encoding") == "utf-8"
 assert seen.get("errors") == "replace"
@@ -114,15 +120,7 @@ def test_child_never_inherits_the_agent_transport_stdin(monkeypatch):
 in 0.07s from a shell. The tool reported a 180s timeout and GitHub triage
 was unusable.
 """
- seen = {}
- real_run = subprocess.run
-
- def spy(*args, **kwargs):
- seen.update(kwargs)
- return real_run(*args, **kwargs)
-
- monkeypatch.setattr(subprocess, "run", spy)
- _shell_tool()(command="echo hello")
+ seen = _spawn_kwargs(monkeypatch)
 
 assert seen["stdin"] is subprocess.DEVNULL
 
diff --git a/tests/unit/test_shell_tools_session.py b/tests/unit/test_shell_tools_session.py
new file mode 100644
index 000000000..f71d141ed
--- /dev/null
+++ b/tests/unit/test_shell_tools_session.py
@@ -0,0 +1,164 @@
+# Copyright(C) 2025-2026 Advanced Micro Devices, Inc. All rights reserved.
+# SPDX-License-Identifier: MIT
+"""run_shell_command shares one session, so state survives between calls (#3380).
+
+Driven through the registered tools rather than the session object, because the
+regression these guard is at the seam: a tool that builds a fresh subprocess per
+call is indistinguishable from one that reuses a session until you ask it where
+it is.
+"""
+
+import os
+
+import pytest
+
+from gaia.agents.tools.shell_tools import ShellToolsMixin
+
+
+class _Host(ShellToolsMixin):
+ """Minimal host: the mixin only needs its own __init__ for rate limiting."""
+
+
+@pytest.fixture
+def tools():
+ """The registered shell tools, keyed by name, on one fresh host."""
+ captured = {}
+ import gaia.agents.base.tools as tools_module
+
+ original = tools_module.tool
+
+ def spy(**kwargs):
+ def decorate(fn):
+ captured[fn.__name__] = fn
+ return original(**kwargs)(fn)
+
+ return decorate
+
+ tools_module.tool = spy
+ try:
+ host = _Host()
+ # No rate limit: these tests run more than three commands in ten seconds.
+ host.max_commands_per_10_seconds = 1000
+ host.max_commands_per_minute = 1000
+ host.register_shell_tools()
+ finally:
+ tools_module.tool = original
+ captured["_host"] = host
+ yield captured
+ host.close_shell_session()
+
+
+def test_cd_in_one_call_is_where_the_next_call_runs(tools, tmp_path):
+ host = tools["_host"]
+ (tmp_path / "workspace").mkdir()
+ host.shell_session.set_cwd(str(tmp_path))
+
+ tools["run_shell_command"](command="cd workspace")
+ result = tools["run_shell_command"](command="pwd")
+
+ assert result["status"] == "success", result
+ assert result["session_cwd"].replace("\\", "/").endswith("/workspace")
+
+
+def test_the_probe_reports_the_session_directory(tools, tmp_path):
+ host = tools["_host"]
+ host.shell_session.set_cwd(str(tmp_path))
+
+ state = tools["get_shell_state"]()
+
+ assert state["status"] == "success"
+ assert state["cwd"] == str(tmp_path.resolve())
+ assert state["environment"] == {}
+
+
+def test_a_variable_set_through_the_tool_reaches_later_commands(tools):
+ assert tools["set_shell_variable"](name="GAIA_TOOL_VAR", value="kept")[
+ "status"
+ ] == ("success")
+
+ state = tools["get_shell_state"]()
+ assert state["environment"]["GAIA_TOOL_VAR"] == "kept"
+
+ echo = "echo %GAIA_TOOL_VAR%" if os.name == "nt" else "echo $GAIA_TOOL_VAR"
+ result = tools["run_shell_command"](command=echo)
+ assert "kept" in result["stdout"]
+
+
+@pytest.mark.parametrize(
+ "name", ["PATH", "PYTHONPATH", "LD_PRELOAD", "dyld_insert_libraries"]
+)
+def test_variables_that_choose_the_binary_are_refused(tools, name):
+ """Setting PATH would let the agent pick which `ls` the whitelist approved."""
+ result = tools["set_shell_variable"](name=name, value="/tmp/evil")
+
+ assert result["status"] == "error"
+ assert tools["get_shell_state"]()["environment"] == {}
+
+
+def test_an_invalid_variable_name_is_refused(tools):
+ result = tools["set_shell_variable"](name="not a name", value="x")
+
+ assert result["status"] == "error"
+
+
+def test_reset_returns_the_session_to_where_it_started(tools, tmp_path):
+ host = tools["_host"]
+ (tmp_path / "sub").mkdir()
+ start = host.shell_session.cwd
+ tools["set_shell_variable"](name="GAIA_TOOL_RESET", value="x")
+ host.shell_session.set_cwd(str(tmp_path / "sub"))
+
+ result = tools["reset_shell_session"]()
+
+ assert result["status"] == "success"
+ assert result["cwd"] == start
+ assert tools["get_shell_state"]()["environment"] == {}
+
+
+def test_working_directory_still_scopes_a_single_call(tools, tmp_path):
+ """The existing argument keeps its meaning: this call only."""
+ host = tools["_host"]
+ before = host.shell_session.cwd
+
+ result = tools["run_shell_command"](command="pwd", working_directory=str(tmp_path))
+
+ assert result["status"] == "success", result
+ assert host.shell_session.cwd == before
+
+
+def test_a_forbidden_directory_change_is_not_absorbed(tools, tmp_path):
+ """`cd` must not become a way around the path policy.
+
+ Without this, the agent could step into a directory it may not read and then
+ open a file by bare name — the per-argument check only sees paths.
+ """
+ host = tools["_host"]
+ (tmp_path / "secrets").mkdir()
+ host.shell_session.set_cwd(str(tmp_path))
+ host._is_path_allowed = lambda path: "secrets" not in path.replace("\\", "/")
+ host._shell_session = None # rebuild the session with the guard attached
+
+ host.shell_session.set_cwd(str(tmp_path))
+ result = tools["run_shell_command"](command="cd secrets")
+
+ assert host.shell_session.cwd == str(tmp_path.resolve())
+ assert "warning" in result
+
+
+def test_the_guardrails_still_refuse_a_blocked_command(tools):
+ """Persistence is about state, not policy (#3380 acceptance criterion 6)."""
+ result = tools["run_shell_command"](command="rm -rf /")
+
+ assert result["status"] == "error"
+ assert result["has_errors"] is True
+
+
+def test_teardown_is_repeatable(tools):
+ host = tools["_host"]
+ tools["run_shell_command"](command="echo hello")
+
+ host.close_shell_session()
+ host.close_shell_session()
+
+ # A closed session is replaced on next use rather than left broken.
+ assert tools["run_shell_command"](command="echo again")["status"] == "success"
From 37151a21031c3f894b45ebb5a4166e7940d6e87e Mon Sep 17 00:00:00 2001
From: Ovtcharov <kovtchar@amd.com>
Date: Sat, 5 Sep 2026 03:56:19 -0700
Subject: [PATCH 3/4] docs(shell): describe the persistent shell session and
 its new tools
Also closes the chat agent's shell session in its teardown path, so the temp
directory goes with the agent rather than waiting on the OS.
---
 docs/sdk/mixins/tool-mixins.mdx &#124; 7 +++
 docs/spec/shell-tools-mixin.mdx &#124; 55 +++++++++++++++++--
 .../chat/python/gaia_agent_chat/agent.py &#124; 10 +++-
 3 files changed, 65 insertions(+), 7 deletions(-)
diff --git a/docs/sdk/mixins/tool-mixins.mdx b/docs/sdk/mixins/tool-mixins.mdx
index 8db710821..564805075 100644
--- a/docs/sdk/mixins/tool-mixins.mdx
+++ b/docs/sdk/mixins/tool-mixins.mdx
@@ -122,8 +122,15 @@ class MyAgent(Agent, ShellToolsMixin):
 self.register_shell_tools()
 # Registers:
 # - run_shell_command(command, working_directory=None, timeout=30)
+ # - get_shell_state()
+ # - set_shell_variable(name, value)
+ # - reset_shell_session()
 ```
 
+Commands share one shell session per agent, so a `cd` or a variable set in one
+call is still in effect for the next. `get_shell_state` reads that state back,
+and `reset_shell_session` returns it to where it started.
+
 ---
 
 ## 9.4 File Search Mixin
diff --git a/docs/spec/shell-tools-mixin.mdx b/docs/spec/shell-tools-mixin.mdx
index 8726fa7ea..11a596fc9 100644
--- a/docs/spec/shell-tools-mixin.mdx
+++ b/docs/spec/shell-tools-mixin.mdx
@@ -19,6 +19,7 @@ ShellToolsMixin provides secure shell command execution with comprehensive rate
 
 **Key Features:**
 - Whitelist-based command security (only safe, read-only commands)
+- One persistent shell session per agent — cwd and environment survive between calls
 - Dual-tier rate limiting (burst + sustained)
 - Path traversal prevention with argument validation
 - Git read-only operations support
@@ -41,21 +42,28 @@ ShellToolsMixin provides secure shell command execution with comprehensive rate
 - Blacklist dangerous commands (rm, chmod, sudo, etc.)
 - Git read-only subcommands only
 - Working directory support
- - Timeout protection (default: 30s)
+ - Timeout protection (default: 30s), killing the command's whole process tree
 
-2. **Rate Limiting**
+2. **Persistent Session State**
+ - One session per agent; `cd` and exported variables carry to the next call
+ - `get_shell_state` reports the directory and the changed environment
+ - `set_shell_variable` sets a variable for the rest of the session
+ - `reset_shell_session` returns the session to its starting state
+ - Serialised access; deterministic teardown with no surviving child
+
+3. **Rate Limiting**
 - Max 10 commands per minute (sustained)
 - Max 3 commands per 10 seconds (burst)
 - Clear wait time messaging
 - Timestamp tracking with deque
 
-3. **Path Security**
+4. **Path Security**
 - Validate working directory access
 - Check arguments for path traversal (.., separators)
 - Resolve relative paths to absolute
 - PathValidator integration
 
-4. **Output Management**
+5. **Output Management**
 - Capture stdout and stderr separately
 - Truncate large outputs (max 10,000 chars)
 - Return execution duration
@@ -106,6 +114,9 @@ class ShellToolsMixin:
 
 Tools provided:
 - run_shell_command: Execute safe shell commands with security checks
+ - get_shell_state: Report the session's directory and changed environment
+ - set_shell_variable: Set a variable for the rest of the session
+ - reset_shell_session: Return the session to its starting state
 """
 
 def __init__(self, *args, **kwargs):
@@ -255,6 +266,42 @@ if cmd_base == "git":
 }
 ```
 
+### Persistent Session
+
+Every command runs in the agent's one `ShellSession`
+([`shell_session.py`](https://github.com/amd/gaia/blob/main/src/gaia/agents/tools/shell_session.py)),
+a port of the C++ toolbelt's `gaia::ShellSession`. What persists is the session
+*state*, not a resident child process: each command runs a generated script that
+restores the session's directory and variables, sources the command from its own
+file, then reports the resulting `pwd` and environment to a side file the session
+absorbs. Nothing outlives the call, so there is no child to orphan.
+
+- **Serialised.** One lock per session, taken for the whole command. A call that
+ cannot take it within its own timeout returns a "session busy" error rather
+ than blocking — a command that outran its tool timeout is still inside the
+ subprocess, and waiting on it forever would wedge the agent.
+- **Timeouts kill the tree.** POSIX kills the process group; Windows uses
+ `taskkill /T`. A timed-out build does not leave its compilers running.
+- **`cd` obeys the path policy.** A directory the agent may not access is not
+ absorbed, and the result carries a warning saying so. Otherwise persistence
+ would be a way to reach a forbidden directory and then read files by bare
+ name, which the per-argument check never sees.
+- **Granted binaries still run as argv.** A skill-granted CLI is never handed to
+ a shell as a string; it gets the session's directory and environment, but its
+ arguments cannot act.
+- **`working_directory` is unchanged.** An explicit per-call directory scopes
+ that call only; the session stays where it was.
+
+On Windows commands run as a `cmd.exe` batch script, so `%VAR%` expands as it
+would at a prompt and a `for` variable is written `%%i`. Point `GAIA_SHELL` at a
+POSIX shell (Git Bash, MSYS, WSL) to run shell scripts instead.
+
+Shell state that is not a directory or a variable — aliases, functions, shell
+options — is not carried. The read-only whitelist has no `export`, so
+`set_shell_variable` is how a variable gets set; it refuses `PATH`,
+`PYTHONPATH`, `LD_*` and the rest of the names that decide which binary or which
+code a later command runs.
+
 ### Path Traversal Prevention
 
 ```python
diff --git a/hub/agents/chat/python/gaia_agent_chat/agent.py b/hub/agents/chat/python/gaia_agent_chat/agent.py
index 214851666..f8285699a 100644
--- a/hub/agents/chat/python/gaia_agent_chat/agent.py
+++ b/hub/agents/chat/python/gaia_agent_chat/agent.py
@@ -2346,9 +2346,9 @@ def save_current_session(self) -> bool:
 def __del__(self):
 """Cleanup when agent is destroyed.
 
- Releases watchdog observers, HTTP session, and the two SQLite
- connections owned by this agent. ``__del__`` is best-effort — Python
- doesn't guarantee it fires on interpreter shutdown — but explicit
+ Releases watchdog observers, HTTP session, the shell session, and the
+ two SQLite connections owned by this agent. ``__del__`` is best-effort —
+ Python doesn't guarantee it fires on interpreter shutdown — but explicit
 close() makes tests deterministic (WAL journals released, file handles
 closed) and avoids leaking Session/connection objects in long-running
 services like the Agent UI backend.
@@ -2372,3 +2372,7 @@ def __del__(self):
 self._scratchpad.close_db()
 except Exception as e:
 logger.error(f"Error closing scratchpad during cleanup: {e}")
+ try:
+ self.close_shell_session()
+ except Exception as e:
+ logger.error(f"Error closing the shell session during cleanup: {e}")
From 39ba816ae55216485581cfb1ef52181e34434ccd Mon Sep 17 00:00:00 2001
From: Ovtcharov <kovtchar@amd.com>
Date: Sat, 5 Sep 2026 05:00:46 -0700
Subject: [PATCH 4/4] test(shell): move the granted-CLI argv guards to the
 session seam
Also registers the session's bandit suppressions and drops the Popen kwargs
dict that hid the call's overload from mypy.
---
 .security-suppressions.json &#124; 16 ++++++--
 src/gaia/agents/tools/shell_session.py &#124; 45 ++++++++++----------
 src/gaia/agents/tools/shell_tools.py &#124; 40 ++++++++++++++----
 tests/unit/test_shell_session.py &#124; 14 ++++---
 tests/unit/test_shell_tools_session.py &#124; 23 +++++++++++
 tests/unit/test_skill_binary_grants.py &#124; 57 +++++++++++++++++++-------
 6 files changed, 141 insertions(+), 54 deletions(-)
diff --git a/.security-suppressions.json b/.security-suppressions.json
index 404253b72..650dbc6a7 100644
--- a/.security-suppressions.json
+++ b/.security-suppressions.json
@@ -6,9 +6,19 @@
 {"path": "src/gaia/hub/native_launcher.py", "rule": "S603", "justification": "native binary path is an installed hub artifact (operator-controlled); args are constructed, not shell"},
 {"path": "src/gaia/mcp/mcp_bridge.py", "rule": "B104", "justification": "binds 0.0.0.0 only on explicit caller opt-in (documented flag), never by default"},
 {
- "path": "src/gaia/agents/tools/shell_tools.py",
- "rule": "B602",
- "justification": "Sandboxed shell tool. Every command (and each pipeline segment) is validated against a whitelist via _validate_command before execution; shell=True is enabled ONLY on Windows so cmd.exe can resolve built-ins (dir/cd/type) and pipes that Git-for-Windows tools rely on. Converting to args-list would break piped/whitelisted commands the tool exists to run."
+ "path": "src/gaia/agents/tools/shell_session.py",
+ "rule": "B404",
+ "justification": "Running shell commands is this module's purpose. The whitelist and path policy that decide what may run stay in shell_tools.py, and apply per pipeline segment before anything reaches here."
+ },
+ {
+ "path": "src/gaia/agents/tools/shell_session.py",
+ "rule": "B603",
+ "justification": "argv is always a constructed list, never shell-interpreted by Python. It names either the generated script this module wrote (which sources a command validated per segment by shell_tools.py) or a skill-granted binary, which is run as argv precisely so its arguments cannot act."
+ },
+ {
+ "path": "src/gaia/agents/tools/shell_session.py",
+ "rule": "B607",
+ "justification": "taskkill is a Windows system binary invoked by name with a pid from this module's own child. The alternative is orphaning the process tree a timed-out command started."
 }
 ]
 }
diff --git a/src/gaia/agents/tools/shell_session.py b/src/gaia/agents/tools/shell_session.py
index d7a0753f4..0630b68f3 100644
--- a/src/gaia/agents/tools/shell_session.py
+++ b/src/gaia/agents/tools/shell_session.py
@@ -58,7 +58,9 @@
 _ENV_MARKER = "---GAIA-ENV---"
 
 #: Variables every shell rewrites on its own. Replaying them would make the
-#: session drift a little further from the parent on every command.
+#: session drift a little further from the parent on every command. PATH is
+#: deliberately absent: it is what virtualenv activation changes, so a session
+#: that dropped it would activate a venv and then not use it.
 _VOLATILE_ENV_NAMES = frozenset(
 {
 "_",
@@ -234,7 +236,8 @@ def _terminate_tree(proc: "subprocess.Popen") -> None:
 proc.kill()
 else:
 try:
- os.killpg(os.getpgid(proc.pid), signal.SIGKILL)
+ # pylint: disable=no-member # POSIX-only; the branch never runs on nt
+ os.killpg(os.getpgid(proc.pid), signal.SIGKILL) # type: ignore[attr-defined]
 except (ProcessLookupError, PermissionError) as exc:
 logger.warning("Could not kill process group for pid %s: %s", proc.pid, exc)
 proc.kill()
@@ -428,7 +431,7 @@ def run_argv(
 )
 try:
 cwd = working_directory or self._state.cwd
- result = self._spawn(argv, cwd, self.effective_env(), timeout, shell=False)
+ result = self._spawn(argv, cwd, self.effective_env(), timeout)
 result.cwd = self._state.cwd
 return result
 finally:
@@ -464,7 +467,7 @@ def _run_locked(
 
 # No cwd/env arguments: the script applies both inside the child, so
 # the calling process is never mutated and sessions cannot collide.
- result = self._spawn(argv, None, None, timeout, shell=False)
+ result = self._spawn(argv, None, None, timeout)
 rejected = self._absorb_state(
 self._read_state(state_file), absorb_cwd=working_directory is None
 )
@@ -621,11 +624,10 @@ def _absorb_state(self, state_text: str, absorb_cwd: bool) -> Optional[str]:
 
 @staticmethod
 def _spawn(
- argv,
+ argv: List[str],
 cwd: Optional[str],
 env: Optional[Dict[str, str]],
 timeout: float,
- shell: bool,
 ) -> ShellResult:
 """Run *argv*, capping the wait and killing the whole tree on expiry.
 
@@ -636,25 +638,24 @@ def _spawn(
 stdout: the command succeeded and its output was silently discarded.
 """
 start = time.monotonic()
- popen_kwargs = {
- "stdout": subprocess.PIPE,
- "stderr": subprocess.PIPE,
+ with subprocess.Popen( # nosec B603 - argv is a list, never shell-parsed
+ argv,
+ stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE,
 # stdin is DEVNULL, never inherited. This process's stdin is the
 # agent transport's pipe — held open and never written to — so a
 # child that reads it blocks forever on input that cannot arrive.
- "stdin": subprocess.DEVNULL,
- "cwd": cwd,
- "env": env,
- "encoding": "utf-8",
- "errors": "replace",
- "shell": shell,
- }
- if os.name != "nt":
- # Its own process group, so a timeout can kill the command's
- # children too rather than just the shell that started them.
- popen_kwargs["start_new_session"] = True
-
- with subprocess.Popen(argv, **popen_kwargs) as proc: # nosec B603
+ stdin=subprocess.DEVNULL,
+ cwd=cwd,
+ env=env,
+ encoding="utf-8",
+ errors="replace",
+ # Its own process group on POSIX, so a timeout can kill the
+ # command's children too rather than just the shell that started
+ # them. Windows has no equivalent; _terminate_tree walks the
+ # parent-child tree with taskkill instead.
+ start_new_session=os.name != "nt",
+ ) as proc:
 try:
 stdout, stderr = proc.communicate(timeout=timeout)
 timed_out = False
diff --git a/src/gaia/agents/tools/shell_tools.py b/src/gaia/agents/tools/shell_tools.py
index e46ef7dd8..4fd4370a0 100644
--- a/src/gaia/agents/tools/shell_tools.py
+++ b/src/gaia/agents/tools/shell_tools.py
@@ -279,6 +279,21 @@ def _operator_check_text(command: str) -> str:
 return " ".join(outer)
 
 
+def _requote_for_posix(segments: list) -> str:
+ """Rebuild the command from its validated tokens, quoted for POSIX sh.
+
+ A session needs a shell — ``cd`` and ``export`` exist nowhere else — but
+ handing sh the raw string would newly let it expand what the checks above
+ read literally: ``~``, ``$VAR``, ``*``. ``cat ~/../secret`` is one token to
+ the path validator and a different file to the shell. Re-quoting the tokens
+ keeps the pipeline and gives the shell back exactly the argv it had before
+ there was a session.
+ """
+ return " &#124; ".join(
+ " ".join(shlex.quote(token) for token in segment) for segment in segments
+ )
+
+
 def _split_pipeline(cmd_parts: list) -> list:
 """Split a shlex-split command on ``&#124;`` into its non-empty segments."""
 segments: list = []
@@ -335,11 +350,12 @@ def _session_cwd_guard(self) -> Optional[Callable[[str], bool]]:
 """
 validator = getattr(self, "path_validator", None)
 if validator is not None:
- return validator.is_path_allowed
- checker = getattr(self, "_is_path_allowed", None)
- if checker is not None:
- return checker
- return None
+ guard: Callable[[str], bool] = validator.is_path_allowed
+ return guard
+ checker: Optional[Callable[[str], bool]] = getattr(
+ self, "_is_path_allowed", None
+ )
+ return checker
 
 @property
 def shell_session(self) -> ShellSession:
@@ -1035,10 +1051,16 @@ def run_shell_command(
 )
 
 session = self.shell_session
- exec_command = command
- if not lone_granted_segment and not session.posix_script:
- # cmd.exe: map Unix names to their built-ins when the
- # Git-for-Windows tools that provide them aren't on PATH.
+ if session.posix_script:
+ exec_command = _requote_for_posix(segments)
+ else:
+ # cmd.exe gets the ORIGINAL string: its quoting rules are not
+ # shlex's, and re-quoting a PowerShell -Command body breaks
+ # it. Unchanged from before the session existed.
+ exec_command = command
+
+ # Map Unix names to cmd built-ins when the Git-for-Windows
+ # tools that provide them aren't on PATH.
 _UNIX_TO_WIN = {
 "ls": "dir",
 "pwd": "cd",
diff --git a/tests/unit/test_shell_session.py b/tests/unit/test_shell_session.py
index ec6bebe8d..81e675778 100644
--- a/tests/unit/test_shell_session.py
+++ b/tests/unit/test_shell_session.py
@@ -20,7 +20,10 @@
 ShellSessionClosed,
 )
 
-IS_WINDOWS = os.name == "nt"
+#: True when the session runs commands as a cmd.exe batch file. Setting
+#: GAIA_SHELL to a POSIX shell exercises the other script flavour on the same
+#: box — worth doing before touching the script generator.
+IS_WINDOWS = os.name == "nt" and not os.environ.get("GAIA_SHELL", "").strip()
 
 
 def export_command(name: str, value: str) -> str:
@@ -107,11 +110,12 @@ def test_inherited_variables_are_not_reported_as_changes(self, session):
 """Only what the session actually diverged, or Windows folds every name."""
 session.run("echo hello")
 
- # pytest rewrites PYTEST_CURRENT_TEST after the session took its
- # baseline, so the session is right to call it changed.
+ # The bug this pins: Windows folds variable names and os.environ
+ # upper-cases them, so comparing raw made every inherited variable both
+ # an override and a removal — the session would replay the whole
+ # environment and unset it at the same time.
 diverged = session.environment()
- diverged.pop("PYTEST_CURRENT_TEST", None)
- assert diverged == {}
+ assert len(diverged) < len(os.environ) / 4, diverged
 assert session.removed_environment() == []
 
 def test_set_env_applies_to_the_next_command(self, session):
diff --git a/tests/unit/test_shell_tools_session.py b/tests/unit/test_shell_tools_session.py
index f71d141ed..5021ec12d 100644
--- a/tests/unit/test_shell_tools_session.py
+++ b/tests/unit/test_shell_tools_session.py
@@ -9,6 +9,7 @@
 """
 
 import os
+import shlex
 
 import pytest
 
@@ -153,6 +154,28 @@ def test_the_guardrails_still_refuse_a_blocked_command(tools):
 assert result["has_errors"] is True
 
 
+@pytest.mark.parametrize(
+ "command, expected",
+ [
+ ("cat ~/../secret", "cat '~/../secret'"),
+ ("echo $HOME", "echo '$HOME'"),
+ ("ls *.py", "ls '*.py'"),
+ ("grep -r 'foo bar' .", "grep -r 'foo bar' ."),
+ ("ls &#124; head -3", "ls &#124; head -3"),
+ ],
+)
+def test_the_posix_shell_gets_argv_back_not_a_string_to_expand(command, expected):
+ """A session needs a shell, but the shell must not re-read the arguments.
+
+ `cat ~/../secret` is one literal token to the path validator and a different
+ file to sh. Re-quoting the validated tokens keeps the pipeline and leaves
+ expansion off, as it was before commands went through a shell at all.
+ """
+ from gaia.agents.tools.shell_tools import _requote_for_posix, _split_pipeline
+
+ assert _requote_for_posix(_split_pipeline(shlex.split(command))) == expected
+
+
 def test_teardown_is_repeatable(tools):
 host = tools["_host"]
 tools["run_shell_command"](command="echo hello")
diff --git a/tests/unit/test_skill_binary_grants.py b/tests/unit/test_skill_binary_grants.py
index 42e4320ec..e30b303d2 100644
--- a/tests/unit/test_skill_binary_grants.py
+++ b/tests/unit/test_skill_binary_grants.py
@@ -1289,25 +1289,52 @@ def decorate(fn):
 return captured["run_shell_command"]
 
 
+class _FakeChild:
+ """Stands in for the spawned process, recording the argv it was given."""
+
+ def __init__(self, args):
+ self.args = args
+ self.returncode = 0
+ self.pid = -1
+
+ def __enter__(self):
+ return self
+
+ def __exit__(self, *exc_info):
+ return False
+
+ def communicate(self, timeout=None): # noqa: ARG002 - signature match
+ return "", ""
+
+ def poll(self):
+ return 0
+
+
 def _run_capturing_subprocess(host, command):
- """Run *command* through the tool, intercepting the subprocess call."""
- import subprocess as subprocess_module
+ """Run *command* through the tool, intercepting the spawned child.
 
- import gaia.agents.tools.shell_tools as shell_module
+ ``ran_directly`` is True when the binary itself was executed, False when the
+ tool handed the command to a shell as a generated script — the distinction
+ the grant path depends on.
+ """
+ import gaia.agents.tools.shell_session as session_module
 
 seen = {}
- real_run = shell_module.subprocess.run
+ real_popen = session_module.subprocess.Popen
 
- def fake_run(args, **kwargs):
+ def fake_popen(args, **kwargs):
 seen["args"] = args
- seen["shell"] = kwargs.get("shell", False)
- return subprocess_module.CompletedProcess(args, 0, "", "")
+ first = os.path.basename(str(args[0])).lower()
+ seen["ran_directly"] = not (
+ first.startswith(("cmd", "sh", "bash", "dash")) and first != "sh.py"
+ )
+ return _FakeChild(args)
 
- shell_module.subprocess.run = fake_run
+ session_module.subprocess.Popen = fake_popen
 try:
 _captured_shell_tool(host)(command=command)
 finally:
- shell_module.subprocess.run = real_run
+ session_module.subprocess.Popen = real_popen
 return seen
 
 
@@ -1315,7 +1342,7 @@ def test_a_granted_cli_is_handed_argv_not_a_shell_string():
 call = _run_capturing_subprocess(
 _Gated("gh"), "gh issue list --search x&#124;echo pwned"
 )
- assert call["shell"] is False, "a granted CLI must not go through cmd.exe"
+ assert call["ran_directly"] is True, "a granted CLI must not go through a shell"
 assert isinstance(call["args"], list)
 # The metacharacter stays inside one argument instead of becoming a pipe.
 assert "x&#124;echo" in call["args"], call["args"]
@@ -1326,22 +1353,22 @@ def test_an_env_var_in_a_granted_write_reaches_the_process_unexpanded():
 call = _run_capturing_subprocess(
 _Gated("gh"), "gh issue comment 1 --body %GITHUB_TOKEN%"
 )
- assert call["shell"] is False
+ assert call["ran_directly"] is True
 assert "%GITHUB_TOKEN%" in call["args"]
 
 
 def test_an_ungranted_command_keeps_the_shell_path():
- """The exemption is for granted CLIs only. `pwd`/`ls` still need cmd.exe on
- Windows to resolve built-ins, and this change must not touch them."""
+ """The exemption is for granted CLIs only. `pwd`/`ls` still need a shell to
+ resolve built-ins, and this change must not touch them."""
 call = _run_capturing_subprocess(_Gated(), "pwd")
- assert call["shell"] is (os.name == "nt")
+ assert call["ran_directly"] is False
 
 
 def test_a_pipeline_is_not_run_as_argv():
 """`cmd_parts` has dropped the `&#124;`, so an argv run of a pipeline would
 silently concatenate two commands into one. Only a lone segment qualifies."""
 call = _run_capturing_subprocess(_Gated("gh"), "gh issue list &#124; head -5")
- assert call["shell"] is (os.name == "nt")
+ assert call["ran_directly"] is False
 
 
 def test_pytest_has_no_write_tier():
</div><div class="naked_ctrl">
<form action="/index.cgi/contrast" method="get" name="gate">
<p><a href="http://altstyle.alfasado.net">AltStyle</a> によって変換されたページ <a href="https://patch-diff.githubusercontent.com/raw/amd/gaia/pull/3400.patch">(-&gt;オリジナル)</a>
/ <label>アドレス: <input type="text" name="naked_post_url" value="https://patch-diff.githubusercontent.com/raw/amd/gaia/pull/3400.patch" size="22" /></label> <label>モード: <select name="naked_post_mode">
<option value="default">デフォルト</option>
<option value="speech">音声ブラウザ</option>
<option value="ruby">ルビ付き</option>
<option value="contrast" selected="selected">配色反転</option>
<option value="larger-text">文字拡大</option>
<option value="mobile">モバイル</option>
</select>
<input type="submit" value="表示" />
</p>
</form>
</div>