Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions openhands-tools/openhands/tools/terminal/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ Raw secret values passed through `env` become session-scoped environment
variables. Use the SDK secret registry when you need command-scoped secret
injection behavior.

Agent terminal processes run at lower CPU priority by default so CPU-heavy
commands do not starve the agent server. Set
`OH_TERMINAL_PROCESS_PRIORITY=none` in the terminal `env` to retain the parent
process priority for benchmarks or priority-sensitive development servers.

## Usage Examples

### Basic Usage
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
"""Platform-specific process priority policy for agent terminals."""

import os
import platform
import shutil
from collections.abc import Mapping


TERMINAL_PROCESS_PRIORITY_ENV = "OH_TERMINAL_PROCESS_PRIORITY"


def should_lower_process_priority(env: Mapping[str, str] | None = None) -> bool:
"""Return whether agent terminal processes should run at lower priority."""
source = os.environ if env is None else env
value = source.get(TERMINAL_PROCESS_PRIORITY_ENV)
return value is None or value.strip().lower() != "none"


def get_process_priority_prefix(
env: Mapping[str, str] | None = None,
) -> tuple[str, ...]:
"""Return an argv prefix that lowers an agent terminal's CPU priority."""
if not should_lower_process_priority(env):
return ()

system = platform.system()
if system == "Darwin":
return ("/usr/sbin/taskpolicy", "-c", "utility")
if system == "Linux":
nice_path = shutil.which("nice")
if nice_path is not None:
return (os.path.abspath(nice_path), "-n", "10")
return ()
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@
from openhands.tools.terminal.metadata import CmdOutputMetadata
from openhands.tools.terminal.terminal import TerminalInterface
from openhands.tools.terminal.terminal.interface import parse_ctrl_key
from openhands.tools.terminal.terminal.process_priority import (
get_process_priority_prefix,
)


logger = get_logger(__name__)
Expand Down Expand Up @@ -151,7 +154,7 @@ def initialize(self) -> None:
env["PS2"] = ""
env["TERM"] = "xterm-256color"

bash_cmd = [resolved_shell_path, "-i"]
bash_cmd = [*get_process_priority_prefix(env), resolved_shell_path, "-i"]

# Create a PTY; give the slave to the child, keep the master
master_fd, slave_fd = pty.openpty()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

from __future__ import annotations

import shlex
import threading
import time
import uuid
Expand All @@ -28,6 +29,9 @@
build_terminal_env,
normalize_terminal_env,
)
from openhands.tools.terminal.terminal.process_priority import (
get_process_priority_prefix,
)
from openhands.tools.terminal.terminal.tmux_terminal import TmuxTerminal


Expand Down Expand Up @@ -167,13 +171,20 @@ def _create_pane(self) -> PooledTmuxTerminal:
"""Create a new PooledTmuxTerminal within the shared session."""
assert self._session is not None

shell_command = "/bin/bash"
shell_command = ["/bin/bash"]
if self.username in ["root", "openhands"]:
shell_command = f"su {self.username} -"
shell_command = ["su", self.username, "-"]

window_command = shlex.join(
(
*get_process_priority_prefix(build_terminal_env(self.env)),
*shell_command,
)
)

window = self._session.new_window(
window_name=f"pane-{len(self._all_panes)}",
window_shell=shell_command,
window_shell=window_command,
start_directory=self.work_dir,
)
active_pane = window.active_pane
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""Tmux-based terminal backend implementation."""

import shlex
import time
import uuid
from collections.abc import Mapping
Expand All @@ -20,6 +21,9 @@
from openhands.tools.terminal.metadata import CmdOutputMetadata
from openhands.tools.terminal.terminal import TerminalInterface
from openhands.tools.terminal.terminal.interface import parse_ctrl_key
from openhands.tools.terminal.terminal.process_priority import (
get_process_priority_prefix,
)


logger = get_logger(__name__)
Expand Down Expand Up @@ -79,12 +83,12 @@ def initialize(self) -> None:
env.setdefault("PAGER", "cat")
# Use a dedicated socket to isolate OpenHands sessions from the user's tmux
self.server = libtmux.Server(socket_name=TMUX_SOCKET_NAME, environment=env)
_shell_command = "/bin/bash"
shell_command = ["/bin/bash"]
if self.username in ["root", "openhands"]:
# This starts a non-login (new) shell for the given user
_shell_command = f"su {self.username} -"
shell_command = ["su", self.username, "-"]

window_command = _shell_command
window_command = shlex.join((*get_process_priority_prefix(env), *shell_command))

logger.debug(f"Initializing tmux terminal with command: {window_command}")
session_name = f"openhands-{self.username}-{uuid.uuid4()}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@
TerminalInterface,
parse_ctrl_key,
)
from openhands.tools.terminal.terminal.process_priority import (
should_lower_process_priority,
)


logger = get_logger(__name__)
Expand Down Expand Up @@ -92,6 +95,8 @@ def initialize(self) -> None:
if self._initialized:
return

env = build_terminal_env(self._env)

startupinfo = None
creationflags = 0
if platform.system() == "Windows":
Expand All @@ -101,8 +106,9 @@ def initialize(self) -> None:
startupinfo.dwFlags |= getattr(subprocess, "STARTF_USESHOWWINDOW", 0)
creationflags = getattr(subprocess, "CREATE_NEW_PROCESS_GROUP", 0)
creationflags |= getattr(subprocess, "CREATE_NO_WINDOW", 0)
if should_lower_process_priority(env):
creationflags |= getattr(subprocess, "BELOW_NORMAL_PRIORITY_CLASS", 0)

env = build_terminal_env(self._env)
env.setdefault("PYTHONIOENCODING", "utf-8")
env.setdefault("PYTHONUTF8", "1")

Expand Down
26 changes: 26 additions & 0 deletions tests/tools/terminal/test_pool_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@
through the executor's __call__ interface.
"""

import platform
import re
import tempfile
import threading
import time

import psutil
import pytest

from openhands.sdk.tool import DeclaredResources
Expand All @@ -33,6 +36,29 @@ def pool_executor():
executor.close()


@pytest.mark.skipif(
platform.system() != "Linux",
reason="Linux niceness is only observable on Linux",
)
def test_pool_commands_inherit_lower_priority(pool_executor) -> None:
parent_priority = int(psutil.Process().nice())
observation = pool_executor(
TerminalAction(
command=(
"python -c 'import os; "
'print("OH_PRIORITY=" + '
"str(os.getpriority(os.PRIO_PROCESS, 0)))'"
)
)
)

assert observation.exit_code == 0
match = re.search(r"OH_PRIORITY=(-?\d+)", observation.text)
assert match is not None, observation.text
expected_priority = min(19, parent_priority + 10)
assert int(match.group(1)) >= expected_priority


class TestDeclaredResources:
def test_pool_mode_opts_out_of_framework_locking(self, pool_executor):
"""In pool mode, declared_resources returns empty keys so the
Expand Down
94 changes: 94 additions & 0 deletions tests/tools/terminal/test_process_priority.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
"""Cross-platform process priority coverage for agent terminals."""

import platform
import re
from pathlib import Path
from typing import Literal

import psutil
import pytest

from openhands.tools.terminal.definition import TerminalAction
from openhands.tools.terminal.terminal import process_priority
from openhands.tools.terminal.terminal.factory import create_terminal_session


@pytest.mark.parametrize(
("system", "expected"),
[
pytest.param(
"Darwin",
("/usr/sbin/taskpolicy", "-c", "utility"),
id="macos-utility-qos",
),
pytest.param("Linux", ("/usr/bin/nice", "-n", "10"), id="linux-niceness"),
pytest.param("Windows", (), id="windows-uses-creation-flags"),
pytest.param("FreeBSD", (), id="unsupported-platform"),
],
)
def test_process_priority_prefix_matches_platform(
monkeypatch: pytest.MonkeyPatch,
system: str,
expected: tuple[str, ...],
) -> None:
monkeypatch.setattr(process_priority.platform, "system", lambda: system)
monkeypatch.setattr(
process_priority.shutil, "which", lambda command: f"/usr/bin/{command}"
)
monkeypatch.setattr(process_priority.os.path, "abspath", lambda path: path)

assert process_priority.get_process_priority_prefix() == expected


def test_linux_priority_falls_back_when_nice_is_unavailable(monkeypatch) -> None:
monkeypatch.setattr(process_priority.platform, "system", lambda: "Linux")
monkeypatch.setattr(process_priority.shutil, "which", lambda _command: None)

assert process_priority.get_process_priority_prefix() == ()


@pytest.mark.parametrize("system", ["Darwin", "Linux"])
def test_none_setting_disables_process_priority_policy(
monkeypatch: pytest.MonkeyPatch,
system: str,
) -> None:
monkeypatch.setattr(process_priority.platform, "system", lambda: system)
monkeypatch.setattr(process_priority.shutil, "which", lambda _: "/usr/bin/nice")
env = {process_priority.TERMINAL_PROCESS_PRIORITY_ENV: "none"}

assert process_priority.get_process_priority_prefix(env) == ()


@pytest.mark.skipif(
platform.system() != "Linux",
reason="Linux niceness is only observable on Linux",
)
@pytest.mark.parametrize("terminal_type", ["tmux", "subprocess"])
def test_linux_terminal_children_inherit_lower_priority(
tmp_path: Path,
terminal_type: Literal["tmux", "subprocess"],
) -> None:
session = create_terminal_session(
work_dir=str(tmp_path),
terminal_type=terminal_type,
)
parent_priority = int(psutil.Process().nice())
try:
session.initialize()
observation = session.execute(
TerminalAction(
command=(
"python -c 'import os; "
'print("OH_PRIORITY=" + '
"str(os.getpriority(os.PRIO_PROCESS, 0)))'"
)
)
)
finally:
session.close()

assert observation.exit_code == 0
match = re.search(r"OH_PRIORITY=(-?\d+)", observation.text)
assert match is not None, observation.text
expected_priority = min(19, parent_priority + 10)
assert int(match.group(1)) >= expected_priority
23 changes: 23 additions & 0 deletions tests/tools/terminal/test_windows_terminal.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from collections.abc import Generator
from typing import cast

import psutil
import pytest
from pydantic import SecretStr

Expand All @@ -19,6 +20,9 @@
from openhands.tools.terminal import TerminalAction, TerminalTool
from openhands.tools.terminal.impl import TerminalExecutor
from openhands.tools.terminal.terminal import TerminalSession, create_terminal_session
from openhands.tools.terminal.terminal.windows_terminal import (
WindowsTerminal,
)


pytestmark = pytest.mark.skipif(
Expand Down Expand Up @@ -93,6 +97,25 @@ def test_basic_command_execution(windows_session) -> None:
assert "Hello from Windows terminal" in obs.text


def test_terminal_process_runs_below_normal_priority(windows_session) -> None:
terminal = cast(WindowsTerminal, windows_session.terminal)
assert terminal.process is not None
try:
observation = windows_session.execute(
TerminalAction(
command=(
'python -c "import psutil; '
"print('OH_PRIORITY=' + str(psutil.Process().nice()))\""
)
)
)
assert observation.exit_code == 0
assert f"OH_PRIORITY={psutil.BELOW_NORMAL_PRIORITY_CLASS}" in observation.text
finally:
terminal.process.terminate()
terminal.process.wait(timeout=5)


@pytest.mark.parametrize(
("command", "expected"),
[
Expand Down