From 430c2718aa96e800539a0f409a3f6878c8ad2ecd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20=C3=84lgamo?= <113307211+Adamact@users.noreply.github.com> Date: Wed, 7 Oct 2026 00:12:27 +0200 Subject: [PATCH] Merge pull request #6532 from Adamact/fix/windows-pwd-import fix(platform): resolve the service home through platform_compat so native Windows can start --- core/platform_compat.py | 20 +++++++++++++ src/agent_tools/web_tools.py | 6 +--- tests/test_platform_compat.py | 42 ++++++++++++++++++++++++++++ tests/test_web_tools_pwd_optional.py | 37 ++++++++++++++++++++++++ website/configuration-reference.md | 4 +-- 5 files changed, 102 insertions(+), 7 deletions(-) create mode 100644 tests/test_web_tools_pwd_optional.py diff --git a/core/platform_compat.py b/core/platform_compat.py index cb135f51a..89a590ba0 100644 --- a/core/platform_compat.py +++ b/core/platform_compat.py @@ -67,6 +67,26 @@ def safe_chmod(path, mode: int) -> bool: return False +# ── Account home ──────────────────────────────────────────────────────────── +def service_home() -> Path: + """Return the account home even when a task overrides ``HOME``. + + POSIX reads the passwd entry for the real uid, so a tool that rewrites + ``HOME`` for a sandboxed child still resolves the service account's own + home. Windows has no passwd database and no ``os.getuid``; ``Path.home()`` + resolves through the user profile there and is already correct, so it is + both the Windows answer and the POSIX fallback. + """ + if IS_WINDOWS: + return Path.home() + import pwd # POSIX-only; imported lazily so this module loads on Windows + + try: + return Path(pwd.getpwuid(os.getuid()).pw_dir) + except (KeyError, OSError): + return Path.home() + + # ── Process detach / liveness / teardown ──────────────────────────────────── def detached_popen_kwargs() -> dict: """Keyword args for :class:`subprocess.Popen` that fully detach a child so diff --git a/src/agent_tools/web_tools.py b/src/agent_tools/web_tools.py index 007717bb5..627ed6c68 100644 --- a/src/agent_tools/web_tools.py +++ b/src/agent_tools/web_tools.py @@ -14,7 +14,6 @@ import tempfile import time import html import hashlib -import pwd import urllib.parse import urllib.request import uuid @@ -32,10 +31,7 @@ _ACTIVE_BROWSER_SESSIONS: set[str] = set() def _service_home() -> Path: """Return the account home even when a task overrides ``HOME``.""" - try: - return Path(pwd.getpwuid(os.getuid()).pw_dir) - except (KeyError, OSError): - return Path.home() + return platform_compat.service_home() def _host_npm_roots() -> list[Path]: diff --git a/tests/test_platform_compat.py b/tests/test_platform_compat.py index ccdbec9fa..c551927ff 100644 --- a/tests/test_platform_compat.py +++ b/tests/test_platform_compat.py @@ -3,6 +3,7 @@ import importlib.util import io import sys +import types from pathlib import Path @@ -320,3 +321,44 @@ def test_run_ssh_command_uses_built_argv(monkeypatch): assert captured["kwargs"]["timeout"] == 7 assert captured["kwargs"]["capture_output"] is True assert captured["kwargs"]["text"] is False + + +class _PwdBlocker: + """Meta-path finder that makes ``import pwd`` fail, as it does on Windows.""" + + def find_spec(self, fullname, path=None, target=None): + if fullname == "pwd": + raise ImportError("No module named 'pwd'") + return None + + +def test_service_home_on_windows_never_imports_pwd(monkeypatch): + monkeypatch.setattr(platform_compat, "IS_WINDOWS", True) + monkeypatch.delitem(sys.modules, "pwd", raising=False) + monkeypatch.setattr(sys, "meta_path", [_PwdBlocker(), *sys.meta_path]) + + assert platform_compat.service_home() == Path.home() + + +def test_service_home_reads_the_passwd_entry_on_posix(monkeypatch, tmp_path): + account_home = tmp_path / "service-account" + fake_pwd = types.SimpleNamespace( + getpwuid=lambda uid: types.SimpleNamespace(pw_dir=str(account_home)) + ) + monkeypatch.setattr(platform_compat, "IS_WINDOWS", False) + monkeypatch.setitem(sys.modules, "pwd", fake_pwd) + monkeypatch.setattr(platform_compat.os, "getuid", lambda: 1000, raising=False) + monkeypatch.setenv("HOME", str(tmp_path / "task-override")) + + assert platform_compat.service_home() == account_home + + +def test_service_home_falls_back_when_the_uid_has_no_passwd_entry(monkeypatch): + def missing(uid): + raise KeyError(uid) + + monkeypatch.setattr(platform_compat, "IS_WINDOWS", False) + monkeypatch.setitem(sys.modules, "pwd", types.SimpleNamespace(getpwuid=missing)) + monkeypatch.setattr(platform_compat.os, "getuid", lambda: 4242, raising=False) + + assert platform_compat.service_home() == Path.home() diff --git a/tests/test_web_tools_pwd_optional.py b/tests/test_web_tools_pwd_optional.py new file mode 100644 index 000000000..5a8d8358f --- /dev/null +++ b/tests/test_web_tools_pwd_optional.py @@ -0,0 +1,37 @@ +"""`pwd` is POSIX-only, so importing it at module scope breaks Windows startup. + +`web_tools` is pulled in by `app.py` -> `routes.chat_routes` -> `src.agent_loop` +-> `src.agent_tools`, so a `ModuleNotFoundError: pwd` there takes down the whole +app at import time rather than degrading. On POSIX the missing module is +simulated, so this runs on the hosts CI actually uses. +""" + +import importlib +import sys + +from tests.helpers.import_state import clear_module, preserve_import_state + +MODULE = "src.agent_tools.web_tools" + + +class _PwdBlocker: + """Meta-path finder that makes `import pwd` fail, as it does on Windows.""" + + def find_spec(self, fullname, path=None, target=None): + if fullname == "pwd": + raise ImportError("No module named 'pwd'") + return None + + +def test_web_tools_imports_on_a_host_without_pwd(): + with preserve_import_state("pwd", MODULE): + blocker = _PwdBlocker() + sys.meta_path.insert(0, blocker) + try: + clear_module("pwd") + clear_module(MODULE) + module = importlib.import_module(MODULE) + finally: + sys.meta_path.remove(blocker) + + assert module.PrivateBrowserTool.__module__ == MODULE diff --git a/website/configuration-reference.md b/website/configuration-reference.md index 98ff1f6b4..47801c7ab 100644 --- a/website/configuration-reference.md +++ b/website/configuration-reference.md @@ -88,9 +88,9 @@ The source tree reads **117** `ODYSSEUS_*` variables: 81 an operator may want to | `ODYSSEUS_BROWSER_MCP_CACHE` | `os.path.join(base_dir, 'data', 'local', 'playwright-mcp-cache')` | `src/builtin_mcp.py:229` | Cache directory handed to the browser MCP server, so its npm download survives a container rebuild. | | `ODYSSEUS_BROWSER_MCP_CALL_TIMEOUT_S` | `'90'` | `src/mcp_manager.py:27` | Upper bound in seconds for one browser MCP tool call. A call that exceeds it fails without being retried. | | `ODYSSEUS_BROWSER_MCP_REQUIRE_CACHE` | `''` | `src/builtin_mcp.py:90` | Truthy refuses to start the browser MCP server unless its npm package is already in the npx cache, instead of installing it at startup. | -| `ODYSSEUS_BROWSER_NAMESPACE` | `'odysseus-ui'` | `src/agent_tools/web_tools.py:100` (+1 more) | Namespace for the detached agent-browser daemon's pid files, so two runtimes on one machine do not terminate each other's browsers. | +| `ODYSSEUS_BROWSER_NAMESPACE` | `'odysseus-ui'` | `src/agent_tools/web_tools.py:96` (+1 more) | Namespace for the detached agent-browser daemon's pid files, so two runtimes on one machine do not terminate each other's browsers. | | `ODYSSEUS_BROWSER_NO_SANDBOX` | `'1'` | `src/builtin_mcp.py:142` | Security-relevant. On by default, adding `--no-sandbox` because the Docker image cannot use the Chromium sandbox. Set 0, false or no to keep it. | -| `ODYSSEUS_BROWSER_SCREENSHOT_DIR` | *unset* | `src/agent_tools/web_tools.py:2666` | Where private-browser screenshots are written. Falls back to the container path, then the system temp directory. | +| `ODYSSEUS_BROWSER_SCREENSHOT_DIR` | *unset* | `src/agent_tools/web_tools.py:2662` | Where private-browser screenshots are written. Falls back to the container path, then the system temp directory. | ### Container and workspace mounts