diff --git a/omnigent/_platform.py b/omnigent/_platform.py index 5117825a32..92fbcf90d8 100644 --- a/omnigent/_platform.py +++ b/omnigent/_platform.py @@ -120,6 +120,88 @@ def resolve_cli_binary(name: str, *, env_var: str | None = None) -> str | None: #: True on macOS specifically (the seatbelt sandbox platform). IS_DARWIN = sys.platform == "darwin" +#: Windows console code page for UTF-8 (``chcp 65001``). +_WINDOWS_UTF8_CP = 65001 + + +def _stream_is_tty(stream: object) -> bool: + """Return whether *stream* is attached to an interactive terminal.""" + try: + return bool(stream.isatty()) # type: ignore[attr-defined] + except (AttributeError, OSError, ValueError): + return False + + +def _set_windows_console_output_utf8() -> None: + """Switch the attached Windows console output code page to UTF-8.""" + import ctypes + + kernel32 = ctypes.windll.kernel32 # type: ignore[attr-defined] + kernel32.SetConsoleOutputCP(_WINDOWS_UTF8_CP) + + +def configure_cli_stdio() -> bool: + """Make CLI stdout/stderr safe for Unicode on legacy Windows code pages. + + Chinese/Japanese Windows consoles often default to GBK (``cp936``). Writing + emoji or ``✓``/``✗`` then raises :exc:`UnicodeEncodeError`, which can crash + ``omnigent setup`` / ``config list`` and tear down the host tunnel reconnect + loop. On native Windows this reconfigures terminal streams to UTF-8 and + redirected streams to replacement-safe output without changing their + encoding, and switches the console output code page when possible. + + Idempotent and best-effort: failures are swallowed so a weird redirected + stream never blocks CLI startup. No-op on POSIX (including WSL). + + :returns: ``True`` when running on native Windows (configuration attempted), + ``False`` otherwise. + """ + if not IS_WINDOWS: + return False + has_console_stream = False + for stream in (sys.stdout, sys.stderr): + reconfigure = getattr(stream, "reconfigure", None) + if reconfigure is None: + continue + is_console = _stream_is_tty(stream) + has_console_stream = has_console_stream or is_console + with suppress(Exception): + if is_console: + reconfigure(encoding="utf-8", errors="replace") + else: + reconfigure(errors="replace") + if has_console_stream: + with suppress(Exception): + _set_windows_console_output_utf8() + return True + + +def safe_console_print( + message: str, + *, + file: object | None = None, + flush: bool = True, +) -> None: + """Print *message* without letting :exc:`UnicodeEncodeError` escape. + + Long-lived loops (the host tunnel) must not reconnect-loop because a + success/failure banner used a glyph the active code page cannot encode. + Falls back to a replacement-encoded line when the primary write fails. + + :param message: Text to print (may include emoji / box-drawing glyphs). + :param file: Stream to write to; defaults to :data:`sys.stdout`. + :param flush: Forwarded to :func:`print`. + """ + stream = sys.stdout if file is None else file + try: + print(message, file=stream, flush=flush) + except UnicodeEncodeError: + encoding = getattr(stream, "encoding", None) or "ascii" + fallback = message.encode(encoding, errors="replace").decode(encoding, errors="replace") + with suppress(Exception): + print(fallback, file=stream, flush=flush) + + #: Non-sensitive Windows environment variables that a spawned omnigent #: subprocess needs to function, for env-passthrough allowlists that otherwise #: assume POSIX names. Python uppercases env keys on Windows, so these match @@ -304,3 +386,39 @@ def resolve_repo_symlink(path: Path) -> Path: if candidate.exists(): return candidate.resolve() return path + + +def static_api_key_print_command(api_key: str) -> str: + """Return a shell command that prints *api_key* with no trailing newline. + + Used as Claude/Codex ``apiKeyHelper`` / gateway auth commands when the + credential is a static secret. POSIX hosts use ``printf %s`` (no newline, + unlike ``echo``). Native Windows ``cmd.exe`` has no ``printf``, so a bare + ``printf`` helper fails and Claude Code silently falls back to + ``~/.claude/settings.json`` tokens — which produced false "rate_limit 429" + errors against leftover coding-plan quotas. On Windows we therefore emit a + ``sys.executable -c …`` command with a base64 payload (shell-safe argv) so + both ``cmd.exe`` and Git Bash print the exact key bytes. + + :param api_key: The bearer / API key to print, e.g. ``"sk-..."``. + :returns: A shell command string suitable for ``apiKeyHelper``. + """ + import base64 + import shlex + + if not IS_WINDOWS: + return f"printf %s {shlex.quote(api_key)}" + payload = base64.b64encode(api_key.encode("utf-8")).decode("ascii") + # Forward slashes: accepted by cmd.exe and safe under Git Bash (where + # ``D:\code`` would otherwise treat ``\c`` as an escape). + # Normalize explicitly instead of relying on Path.as_posix(): Linux CI + # exercises this branch with a mocked Windows path, which pathlib treats + # as an opaque POSIX filename and therefore leaves backslashes unchanged. + exe_path = str(sys.executable).replace("\\", "/") + exe = f'"{exe_path}"' + # argv[1] is base64 (safe charset); -c string stays ASCII-only. + return ( + f"{exe} -c " + f'"import base64,sys;sys.stdout.buffer.write(base64.b64decode(sys.argv[1]))" ' + f"{payload}" + ) diff --git a/omnigent/claude_native.py b/omnigent/claude_native.py index b15ad8fdc8..49040b33f7 100644 --- a/omnigent/claude_native.py +++ b/omnigent/claude_native.py @@ -15,7 +15,6 @@ import os import re import secrets -import shlex import shutil import signal import subprocess @@ -47,6 +46,7 @@ from websockets.frames import Close from omnigent._native_resume_hint import echo_native_resume_hint +from omnigent._platform import static_api_key_print_command from omnigent._runner_startup import RunnerStartupProgress, runner_startup_progress from omnigent._startup_profile import StartupProfiler from omnigent._terminal_picker_theme import ( @@ -1723,13 +1723,13 @@ def _provider_config_for_native_claude(entry: ProviderEntry) -> ClaudeNativeUcod ) return None # Token delivery mirrors the claude-sdk executor: a dynamic auth_command - # is used verbatim; a static key becomes a ``printf`` apiKeyHelper (the + # is used verbatim; a static key becomes a platform-safe apiKeyHelper (the # runner env allowlist excludes ANTHROPIC_API_KEY, so the key must reach # Claude Code via the helper, not the environment). if family.auth_command: api_key_helper = family.auth_command elif family.api_key: - api_key_helper = f"printf %s {shlex.quote(family.api_key)}" + api_key_helper = static_api_key_print_command(family.api_key) else: _logger.warning( "native-claude: provider %r is the Claude default but has no usable " diff --git a/omnigent/cli.py b/omnigent/cli.py index 3d01129c10..218faafcbe 100644 --- a/omnigent/cli.py +++ b/omnigent/cli.py @@ -28,7 +28,7 @@ from rich.console import Console from rich.table import Table -from omnigent._platform import IS_WINDOWS, resolve_repo_symlink +from omnigent._platform import IS_WINDOWS, configure_cli_stdio, resolve_repo_symlink from omnigent.cli_common import ( RESUME_PICKER_SENTINEL as _RESUME_PICKER_SENTINEL, ) @@ -1596,6 +1596,11 @@ def main() -> None: so unhandled exceptions are captured even when the user didn't enable ``--log`` or ``--debug-events``. """ + # Windows GBK consoles raise UnicodeEncodeError on emoji / ✓ / ✗ and can + # tear down setup/config/host. Do this before any user-facing print + # (crash handler, Rich, click). No-op on POSIX. + configure_cli_stdio() + # Friendly crash handler: replaces Python's raw traceback with a # calm, branded crash screen + a one-tap path to file a GitHub issue # (browser opens the repo's pre-filled bug-report template with the @@ -2690,6 +2695,9 @@ def _build_host_daemon_env( for key, value in os.environ.items() if key in _RUNNER_ENV_ALLOWLIST or key.startswith(daemon_env_prefixes) } + # The daemon owns this binary log file. Keep raw stdout/stderr aligned with + # the UTF-8 logging handler instead of mixing locale-encoded print output. + env["PYTHONIOENCODING"] = "utf-8:replace" return env diff --git a/omnigent/codex_native_app_server.py b/omnigent/codex_native_app_server.py index 90c7beb6b0..5e7c1d5022 100644 --- a/omnigent/codex_native_app_server.py +++ b/omnigent/codex_native_app_server.py @@ -23,6 +23,7 @@ if TYPE_CHECKING: from omnigent.onboarding.provider_config import ProviderEntry +from omnigent._platform import static_api_key_print_command from omnigent.codex_native_bridge import write_policy_hook_config from omnigent.codex_native_process_registry import ( CodexNativeProcessOwnerLock, @@ -1369,7 +1370,7 @@ def _codex_provider_launch(entry: ProviderEntry, model: str | None) -> NativeCod if family.auth_command: auth_command = family.auth_command elif family.api_key: - auth_command = f"printf %s {shlex.quote(family.api_key)}" + auth_command = static_api_key_print_command(family.api_key) else: # Serves openai but carries no usable credential. return None diff --git a/omnigent/host/_daemon_entry.py b/omnigent/host/_daemon_entry.py index 662dad6872..fec5715553 100644 --- a/omnigent/host/_daemon_entry.py +++ b/omnigent/host/_daemon_entry.py @@ -28,6 +28,10 @@ def main() -> None: :raises SystemExit: If neither / both of ``--server`` and ``--local`` are provided. """ + from omnigent._platform import configure_cli_stdio + + configure_cli_stdio() + parser = argparse.ArgumentParser( description="Background host daemon", ) diff --git a/omnigent/host/connect.py b/omnigent/host/connect.py index a3553cd6e8..f61c43c5b2 100644 --- a/omnigent/host/connect.py +++ b/omnigent/host/connect.py @@ -22,7 +22,7 @@ import websockets.asyncio.client from websockets.exceptions import InvalidStatus, InvalidURI -from omnigent._platform import WINDOWS_ENV_PASSTHROUGH +from omnigent._platform import WINDOWS_ENV_PASSTHROUGH, safe_console_print from omnigent.env_credentials import env_names_with_omnigent_prefix from omnigent.harness_aliases import canonicalize_harness from omnigent.harness_availability import HARNESS_BINARY_MISSING, HarnessAvailability @@ -178,6 +178,11 @@ def _display_log_path(path: Path) -> str: _ORPHAN_REAP_INTERVAL_S = 2.0 +def _orphan_reaping_supported() -> bool: + """Return whether this platform exposes the POSIX child-wait primitives.""" + return hasattr(os, "waitpid") and hasattr(os, "WNOHANG") + + def _install_child_subreaper() -> bool: """Make this process reap orphaned descendants (Linux only). @@ -796,6 +801,8 @@ def _reap_orphans_once(self) -> int: :returns: Count of orphan (non-runner) processes reaped this sweep. """ + if not _orphan_reaping_supported(): + return 0 if self._owned_subprocess_ops > 0: # A host-owned subprocess (e.g. a git worktree command) is running # in a worker thread. Its child is a DIRECT child of this process @@ -1018,11 +1025,10 @@ def _fatal_upgrade_error(self, exc: InvalidURI | InvalidStatus) -> HostConnectEr # terminal — print once per redirect streak so a foreground # `omnigent host` shows the auth problem and its fix instead # of sitting silent while it retries. - print( + safe_console_print( f"⚠ {cause} Retrying — this also happens briefly while " f"the server restarts. {self._credentials_fix_hint()}", file=sys.stderr, - flush=True, ) return None return self._classify_http_status(exc.response.status_code) @@ -1194,11 +1200,10 @@ async def _handle_launch( # host's own terminal shows lifecycle lines, but the runner's real # output — the agent turn, tracebacks — lands only in this file. session_line = f"\n session: {frame.session_id}" if frame.session_id else "" - print( + safe_console_print( f" ↑ Runner started: {runner_id} (pid={proc.pid})\n" f" log: {_display_log_path(log_path)}" f"{session_line}", - flush=True, ) return HostLaunchRunnerResultFrame( request_id=frame.request_id, @@ -1232,9 +1237,8 @@ def _handle_stop( handle.proc.kill() handle.proc.wait() _logger.info("Stopped runner %s", frame.runner_id) - print( + safe_console_print( f" ↓ Runner stopped: {frame.runner_id}", - flush=True, ) return HostStopRunnerResultFrame( request_id=frame.request_id, @@ -1915,11 +1919,12 @@ async def run(self) -> None: # runner dies (this host is PID 1 in a container, or a subreaper # otherwise). Without this they pile up as zombies and can # OOM the box on a long-blocked run (#1782). - if _install_child_subreaper(): - _logger.debug("installed PR_SET_CHILD_SUBREAPER; host will reap orphans") - self._reaper_task = asyncio.create_task( - self._orphan_reaper_loop(), name="host-orphan-reaper" - ) + if _orphan_reaping_supported(): + if _install_child_subreaper(): + _logger.debug("installed PR_SET_CHILD_SUBREAPER; host will reap orphans") + self._reaper_task = asyncio.create_task( + self._orphan_reaper_loop(), name="host-orphan-reaper" + ) backoff = _RECONNECT_BASE_S try: while True: @@ -2187,15 +2192,14 @@ async def _serve_frames(self, ws: websockets.asyncio.client.ClientConnection) -> for runner_id, error in list(self._unreported_exits.items()): del self._unreported_exits[runner_id] await self._report_runner_exit(runner_id, error) - # ``print`` (not ``_logger.warning``) so the user always sees the - # success line after the noisy ``databricks.sdk`` warnings — - # otherwise the terminal goes silent after auth and there's no - # signal the WS handshake actually completed. - print( + # User-facing banner (not ``_logger``) so the line is visible after + # noisy ``databricks.sdk`` warnings. ``safe_console_print`` so a + # legacy Windows code page cannot UnicodeEncodeError-out of the + # tunnel loop and trigger reconnect flaps. + safe_console_print( f"✓ Connected as {self._identity.name!r} " f"({self._identity.host_id}), {len(hello.runners)} live runner(s). " "Listening for sessions — Ctrl-C to disconnect.", - flush=True, ) loop = asyncio.get_running_loop() @@ -2356,20 +2360,20 @@ def run_host_process( path = config_path or CONFIG_PATH identity = load_or_create_host_identity(path) if not path.exists(): - print(f"Auto-generated {path} ({identity.host_id}, name: {identity.name})") - print(f"Connecting to {server_url} as {identity.name!r} ({identity.host_id})") + safe_console_print(f"Auto-generated {path} ({identity.host_id}, name: {identity.name})") + safe_console_print(f"Connecting to {server_url} as {identity.name!r} ({identity.host_id})") # Tell the user where logs land up front — `omnigent host` used to run # silently, so a stuck/quiet host gave no hint where to look. Session # work goes to per-runner files under the runner dir (the exact # file is printed when each runner launches). The host process's # own diagnostics go to the host destination. - print(f"Session logs: {_display_log_path(_runner_log_dir())}/") - print(f"This host's log: {_display_log_path(host_log_path)}") + safe_console_print(f"Session logs: {_display_log_path(_runner_log_dir())}/") + safe_console_print(f"This host's log: {_display_log_path(host_log_path)}") from omnigent.cli_diagnostics import current_cli_log_path _cli_log = current_cli_log_path() if _cli_log is not None and _cli_log != host_log_path: - print(f"CLI diagnostics: {_display_log_path(_cli_log)}") + safe_console_print(f"CLI diagnostics: {_display_log_path(_cli_log)}") host = HostProcess(identity, server_url) try: @@ -2378,5 +2382,8 @@ def run_host_process( # Fail loud: a permanent connection failure must not look like the # process is still working. Print the cause + fix, then exit non-zero # instead of the old behavior of reconnecting silently forever. - print(f"\n✗ Could not connect to {server_url}.\n{exc}", file=sys.stderr, flush=True) + safe_console_print( + f"\n✗ Could not connect to {server_url}.\n{exc}", + file=sys.stderr, + ) raise SystemExit(1) from exc diff --git a/omnigent/inner/claude_sdk_executor.py b/omnigent/inner/claude_sdk_executor.py index 618ba8fb06..2284fca5d0 100644 --- a/omnigent/inner/claude_sdk_executor.py +++ b/omnigent/inner/claude_sdk_executor.py @@ -37,12 +37,12 @@ import tempfile import time from collections.abc import AsyncIterator, Awaitable, Callable, Iterator -from contextlib import contextmanager, suppress +from contextlib import contextmanager, nullcontext, suppress from dataclasses import dataclass from types import ModuleType from typing import Any, Protocol, TypeAlias, cast -from omnigent._platform import resolve_cli_binary, stable_user_id +from omnigent._platform import IS_WINDOWS, resolve_cli_binary, stable_user_id from omnigent.inner import _proc from omnigent.inner.bundle_skills import ensure_bundle_plugin_manifest from omnigent.llms._usage_observer import notify_from_dict as _notify_usage_from_dict @@ -636,6 +636,71 @@ def _unset_env_var(name: str) -> Iterator[None]: os.environ[name] = previous +def _cli_settings_for_api_key_helper( + api_key_helper: str, + *, + base_url: str | None = None, + isolate_user_auth: bool = False, +) -> str: + """Build Claude CLI ``--settings`` JSON for an Omnigent apiKeyHelper. + + Every helper is passed through invocation-local settings. When Omnigent + manages the gateway, the same higher-precedence payload also blanks user + auth and pins the configured base URL so ``~/.claude/settings.json`` cannot + redirect the session. + + :param api_key_helper: Shell command that prints the bearer token. + :param base_url: Gateway base URL to pin when *isolate_user_auth* is true. + :param isolate_user_auth: Isolate the managed gateway from user auth env. + :returns: Compact JSON string for ``ClaudeAgentOptions.settings``. + :raises ValueError: If managed gateway isolation has no base URL. + """ + settings: dict[str, object] = {"apiKeyHelper": api_key_helper} + if isolate_user_auth: + if not isinstance(base_url, str) or not base_url: + raise ValueError("Managed Claude gateway auth requires ANTHROPIC_BASE_URL") + settings["env"] = { + # Empty string = unset for provider selection (Claude Code docs). + "ANTHROPIC_AUTH_TOKEN": "", + "ANTHROPIC_API_KEY": "", + "ANTHROPIC_BASE_URL": base_url, + } + return json.dumps(settings, separators=(",", ":")) + + +def _pin_cli_settings_base_url(options: SdkOptions, base_url: str) -> None: + """Rewrite ``options.settings`` env so ``ANTHROPIC_BASE_URL`` matches *base_url*. + + The gateway shim rewrites ``options.env`` after settings JSON is built; + without this pin, a higher-precedence ``--settings`` ``env`` block would + keep the upstream URL and bypass the shim. Gateway settings are generated + by Omnigent, so an invalid shape is a configuration error rather than a + condition to ignore. + + :param options: SDK options whose ``settings`` string may carry ``env``. + :param base_url: Final base URL the CLI must use, e.g. the shim loopback. + :raises RuntimeError: If managed gateway settings cannot be updated safely. + """ + settings = getattr(options, "settings", None) + if not isinstance(settings, str) or not settings: + raise RuntimeError("Claude gateway SDK options are missing managed settings") + try: + body = json.loads(settings) + except json.JSONDecodeError as exc: + raise RuntimeError("Claude gateway SDK settings are not valid JSON") from exc + if ( + not isinstance(body, dict) + or not isinstance(body.get("apiKeyHelper"), str) + or not body["apiKeyHelper"] + ): + raise RuntimeError("Claude gateway SDK settings are missing apiKeyHelper") + settings_env = body.get("env") + if not isinstance(settings_env, dict): + raise RuntimeError("Claude gateway SDK settings are missing managed auth env") + settings_env["ANTHROPIC_BASE_URL"] = base_url + options.settings = json.dumps(body, separators=(",", ":")) + + _CLOSE_ATTR: str = "close" _TRANSPORT_ATTR: str = "transport" _ACLOSE_ATTR: str = "aclose" @@ -783,6 +848,52 @@ async def handler(args: ToolArgs) -> McpResponse: return mcp_tools +# Keep large prompts out of CLI argv; Windows CreateProcess has a +# particularly small command-line limit. +_CLI_INLINE_SYSTEM_PROMPT_MAX_CHARS = 4000 + + +def _system_prompt_for_cli( + system_prompt: str | None, + *, + spill_dir: pathlib.Path | None = None, +) -> str | dict[str, str] | None: + """Return a ClaudeAgentOptions ``system_prompt`` value safe for CLI argv. + + Short prompts stay inline. On Windows, or when the prompt exceeds + :data:`_CLI_INLINE_SYSTEM_PROMPT_MAX_CHARS`, write the text to a temp + file and return ``{"type": "file", "path": ...}`` so the Claude Agent + SDK passes ``--system-prompt-file`` instead of embedding the body in + the CreateProcess command line. + + :param system_prompt: Assembled system instructions, or ``None``. + :param spill_dir: Optional directory for spilled files; defaults to + the process temp dir. + :returns: The original string, a ``SystemPromptFile`` dict, or ``None``. + """ + if system_prompt is None: + return None + if not system_prompt: + return system_prompt + if not IS_WINDOWS and len(system_prompt) <= _CLI_INLINE_SYSTEM_PROMPT_MAX_CHARS: + return system_prompt + directory = spill_dir if spill_dir is not None else pathlib.Path(tempfile.gettempdir()) + directory.mkdir(parents=True, exist_ok=True) + # delete=False: the Claude CLI reads the path after we return; we unlink + # when the Omnigent session closes (see ClaudeSDKExecutor.close_session). + with tempfile.NamedTemporaryFile( + mode="w", + encoding="utf-8", + suffix=".md", + prefix="omnigent-claude-system-prompt-", + dir=directory, + delete=False, + ) as handle: + handle.write(system_prompt) + path = handle.name + return {"type": "file", "path": path} + + def _augment_system_prompt_for_omnigent_mcp_tools( system_prompt: str, tool_schemas: list[ToolSpec], @@ -1415,6 +1526,11 @@ def __init__( # Force-close tasks for clients evicted on turn cancellation, kept # referenced so they are not GC'd mid-close. self._cancel_close_tasks: set[asyncio.Task[None]] = set() + # Per-session source text and CLI-safe prompt value. + self._cli_system_prompts: dict[ + str, + tuple[str, str | dict[str, str] | None], + ] = {} # Prefer system-installed claude over the SDK's bundled CLI. # The bundled CLI may be older and send beta flags that the @@ -1498,6 +1614,56 @@ def __del__(self) -> None: if getattr(self, "_cli_wrapper_path", None): with suppress(Exception): pathlib.Path(self._cli_wrapper_path).unlink(missing_ok=True) + for _source, value in getattr(self, "_cli_system_prompts", {}).values(): + if isinstance(value, dict) and value.get("type") == "file": + with suppress(Exception): + pathlib.Path(value["path"]).unlink(missing_ok=True) + + def _resolve_cli_system_prompt( + self, + session_key: str, + system_prompt: str, + ) -> str | dict[str, str] | None: + """Return a CLI-safe system prompt, spilling to a file when needed. + + Caches per *session_key* so multi-turn reconnects reuse one spill + file instead of leaking a tempfile every turn. + + :param session_key: Omnigent session id used as the cache key. + :param system_prompt: Assembled system instructions for this turn. + :returns: Inline string, ``SystemPromptFile`` dict, or ``None``. + """ + cached = self._cli_system_prompts.get(session_key) + if cached is not None: + cached_source, cached_value = cached + if cached_source == system_prompt: + return cached_value + if isinstance(cached_value, dict) and cached_value.get("type") == "file": + should_spill = bool(system_prompt) and ( + IS_WINDOWS or len(system_prompt) > _CLI_INLINE_SYSTEM_PROMPT_MAX_CHARS + ) + if should_spill: + pathlib.Path(cached_value["path"]).write_text( + system_prompt, + encoding="utf-8", + ) + self._cli_system_prompts[session_key] = ( + system_prompt, + cached_value, + ) + return cached_value + pathlib.Path(cached_value["path"]).unlink(missing_ok=True) + resolved = _system_prompt_for_cli(system_prompt or None) + self._cli_system_prompts[session_key] = (system_prompt, resolved) + return resolved + + def _forget_cli_system_prompt(self, session_key: str) -> None: + """Drop and unlink any spilled system-prompt file for *session_key*.""" + cached = self._cli_system_prompts.pop(session_key, None) + value = cached[1] if cached is not None else None + if isinstance(value, dict) and value.get("type") == "file": + with suppress(OSError): + pathlib.Path(value["path"]).unlink(missing_ok=True) async def _route_options_through_gateway_shim(self, options: SdkOptions) -> None: """ @@ -1525,10 +1691,16 @@ async def _route_options_through_gateway_shim(self, options: SdkOptions) -> None "ClaudeSDKExecutor(gateway=True) built SDK options without " "env['ANTHROPIC_BASE_URL']; cannot route through the gateway shim." ) + # Validate managed settings before allocating the loopback listener. + # Pinning the current upstream is a no-op for routing and ensures an + # invalid payload fails without leaving a started shim behind. + _pin_cli_settings_base_url(options, env["ANTHROPIC_BASE_URL"]) if self._gateway_shim is None: self._gateway_shim = ClaudeGatewayShim(upstream_base_url=env["ANTHROPIC_BASE_URL"]) await self._gateway_shim.start() env["ANTHROPIC_BASE_URL"] = self._gateway_shim.base_url + # ``--settings`` env outranks process env; keep it on the shim URL. + _pin_cli_settings_base_url(options, self._gateway_shim.base_url) async def _get_or_create_client( self, @@ -1559,13 +1731,15 @@ def _tee_stderr(line: str) -> None: # error. The SDK merges ``os.environ`` with ``options.env``, # so we unset in ``os.environ`` for the spawn window. # - # ANTHROPIC_API_KEY is also stripped so the CLI uses its - # subscription auth rather than a developer API key that - # would charge separately. Safe even in Databricks mode: - # ``options.settings`` explicitly sets apiKeyHelper and - # ``options.env`` sets the Databricks base URL, so the - # Claude CLI does not need an inherited Anthropic key. - with _unset_env_var("CLAUDECODE"), _unset_env_var("ANTHROPIC_API_KEY"): + # Managed gateways strip ANTHROPIC_API_KEY / + # ANTHROPIC_AUTH_TOKEN so parent-shell exports cannot bypass + # apiKeyHelper. Non-gateway sessions retain Claude Code's + # native auth behavior. + with ( + _unset_env_var("CLAUDECODE"), + _unset_env_var("ANTHROPIC_API_KEY") if self._gateway else nullcontext(), + _unset_env_var("ANTHROPIC_AUTH_TOKEN") if self._gateway else nullcontext(), + ): await asyncio.wait_for(client.connect(), timeout=_CONNECT_TIMEOUT_SECONDS) except asyncio.TimeoutError as exc: await self._force_close_client(client) @@ -1622,6 +1796,7 @@ def _tee_stderr(line: str) -> None: async def close_session(self, session_key: str) -> None: self._crashed_sessions.pop(session_key, None) + self._forget_cli_system_prompt(session_key) await self._close_live_client(session_key) async def _close_live_client(self, session_key: str) -> None: @@ -1673,7 +1848,7 @@ def _evict_client_on_cancel(self, session_key: str) -> None: task.add_done_callback(self._cancel_close_tasks.discard) async def close(self) -> None: - session_keys = list(self._clients) + session_keys = set(self._clients) | set(self._cli_system_prompts) for session_key in session_keys: await self.close_session(session_key) if self._gateway_shim is not None: @@ -2123,8 +2298,15 @@ async def run_turn( # ``""`` here would still leave an empty key in the child env. env = dict(self._extra_env) api_key_helper = env.pop(_CLAUDE_API_KEY_HELPER_ENV_KEY, None) + # Every Omnigent helper is invocation-local. Only a managed gateway + # additionally overrides user auth and base URL; ordinary Claude + # sessions retain their native ~/.claude/settings.json behavior. settings_payload = ( - json.dumps({"apiKeyHelper": api_key_helper}, separators=(",", ":")) + _cli_settings_for_api_key_helper( + api_key_helper, + base_url=env.get("ANTHROPIC_BASE_URL"), + isolate_user_auth=self._gateway, + ) if api_key_helper else None ) @@ -2187,7 +2369,7 @@ def _on_stderr(line: str) -> None: bundle_plugins.append({"type": "local", "path": str(self._bundle_dir)}) options_kwargs: dict[str, Any] = { # type: ignore[explicit-any] # ClaudeAgentOptions accepts mixed-typed kwargs (str / list / dict / callable / etc.) "tools": base_tools, - "system_prompt": system_prompt or None, + "system_prompt": self._resolve_cli_system_prompt(session_key, system_prompt), "mcp_servers": mcp_servers if mcp_servers else {}, "allowed_tools": allowed_tools, "permission_mode": self._permission_mode, diff --git a/omnigent/inner/codex_executor.py b/omnigent/inner/codex_executor.py index 50e1f0a5b1..7eb4d2996c 100644 --- a/omnigent/inner/codex_executor.py +++ b/omnigent/inner/codex_executor.py @@ -22,7 +22,7 @@ from pathlib import Path from typing import Any, Protocol, TypeAlias -from omnigent._platform import resolve_cli_binary +from omnigent._platform import IS_WINDOWS, resolve_cli_binary from omnigent.llms._usage_observer import notify_from_dict as _notify_usage_from_dict from omnigent.reasoning_effort import CODEX_EFFORTS, validate_effort from omnigent.runner.identity import OMNIGENT_SESSION_ENV_VAR @@ -810,6 +810,7 @@ def _databricks_codex_config_overrides( base_url: str, auth_command: str, auth_refresh_interval_ms: int | None = None, + platform_auth_shell: bool = False, ) -> list[str]: """Return TOML-fragment overrides for the Codex per-conversation config. @@ -821,10 +822,15 @@ def _databricks_codex_config_overrides( a bearer token, e.g. ``"databricks auth token --host ..."``. :param auth_refresh_interval_ms: Refresh cadence in milliseconds, e.g. ``900000``. + :param platform_auth_shell: Use the native platform shell for a + caller-supplied generic auth command. :returns: Codex TOML-fragment override strings. """ provider_name = "omnigent_databricks" - auth_command_json = json.dumps(auth_command) + auth_program, auth_args = _codex_auth_process( + auth_command, + platform_auth_shell=platform_auth_shell, + ) return [ f"model={json.dumps(model)}", f'model_provider="{provider_name}"', @@ -833,8 +839,8 @@ def _databricks_codex_config_overrides( "model_providers.omnigent_databricks=" '{name="Omnigent Databricks",' f"base_url={json.dumps(base_url)}," - 'auth={command="sh",' - f'args=["-c",{auth_command_json}],' + f"auth={{command={json.dumps(auth_program)}," + f"args={json.dumps(auth_args)}," "timeout_ms=5000," f"refresh_interval_ms={auth_refresh_interval_ms or _GATEWAY_AUTH_REFRESH_MS}" "}," @@ -875,7 +881,12 @@ def _provider_codex_config_overrides( :returns: Codex TOML-fragment override strings. """ provider_name = "omnigent_provider" - auth_command_json = json.dumps(auth_command) + auth_program, auth_args = _codex_auth_process( + auth_command, + platform_auth_shell=True, + ) + auth_program_json = json.dumps(auth_program) + auth_args_json = json.dumps(auth_args) # codex >= 0.137 removed the chat/completions wire from its config schema: # any provider block carrying wire_api="chat" makes codex hard-fail config # load ("wire_api = \"chat\" is no longer supported"), which broke OSS / @@ -894,8 +905,8 @@ def _provider_codex_config_overrides( f"model_providers.{provider_name}=" '{name="Omnigent Provider",' f"base_url={json.dumps(base_url)}," - 'auth={command="sh",' - f'args=["-c",{auth_command_json}],' + f"auth={{command={auth_program_json}," + f"args={auth_args_json}," "timeout_ms=5000," f"refresh_interval_ms={_GATEWAY_AUTH_REFRESH_MS}" "}," @@ -904,6 +915,20 @@ def _provider_codex_config_overrides( return overrides +def _codex_auth_process( + auth_command: str, + *, + platform_auth_shell: bool, +) -> tuple[str, list[str]]: + """Return the Codex auth executable and argv for *auth_command*.""" + if platform_auth_shell and IS_WINDOWS: + return ( + "powershell.exe", + ["-NoProfile", "-NonInteractive", "-Command", f"& {auth_command}"], + ) + return "sh", ["-c", auth_command] + + def _parse_optional_int(value: str | None) -> int | None: """Parse an optional integer env-var value. @@ -2223,6 +2248,7 @@ def __init__( self._gateway_uses_databricks_profile = False if gateway: host = self._gateway_host + platform_auth_shell = False # ``effective_model`` resolves to a concrete model for the codex # config. On the Databricks-profile-derivation branch (no gateway # host or base URL supplied directly) a ``databricks-*`` default is @@ -2270,6 +2296,7 @@ def __init__( ) base_url = base_url_override auth_command = gateway_auth_command + platform_auth_shell = True if model is None: # Directly-supplied neutral gateway: the Omnigent producer always # resolves a concrete model (spec > provider default > @@ -2290,6 +2317,7 @@ def __init__( base_url=base_url, auth_command=auth_command, auth_refresh_interval_ms=self._gateway_auth_refresh_interval_ms, + platform_auth_shell=platform_auth_shell, ) ) if not enable_web_search: diff --git a/omnigent/inner/loader.py b/omnigent/inner/loader.py index a498685154..24825d6587 100644 --- a/omnigent/inner/loader.py +++ b/omnigent/inner/loader.py @@ -105,7 +105,7 @@ def load_agent_def( """ if isinstance(path_or_dict, (str, Path)): path = Path(path_or_dict) - with open(path) as f: + with path.open(encoding="utf-8") as f: data = yaml.load(f, Loader=_OmnigentYamlLoader) instructions_root: Path | None = path.parent else: @@ -176,7 +176,7 @@ def _read_contained_file(root: Path, value: str) -> str | None: try: resolved = candidate.resolve() if resolved.is_relative_to(root.resolve()) and resolved.is_file(): - return resolved.read_text() + return resolved.read_text(encoding="utf-8") except OSError: # Path too long or invalid characters — fall through to inline text. pass diff --git a/omnigent/inner/os_env.py b/omnigent/inner/os_env.py index a38a928472..c4f1f61214 100644 --- a/omnigent/inner/os_env.py +++ b/omnigent/inner/os_env.py @@ -441,6 +441,11 @@ def _start_locked(self) -> None: parent_env=credential_parent_env, ) env.update(credential_runtime.helper_env_updates) + elif IS_WINDOWS: + # Windows delivers helper config via ``--config-file`` (no + # ``pass_fds``). Inactive sandboxes still need a private scratch + # dir for that ephemeral file; POSIX uses a pipe and skips this. + self._tmpdir = create_private_tmpdir() # Start L7 egress proxy if rules are configured. The proxy # listens on a Unix socket in the scratch tmpdir; the helper @@ -490,9 +495,12 @@ def _start_locked(self) -> None: # ``pass_fds`` / fd inheritance in ``subprocess`` (CPython rejects # ``pass_fds`` outright there), so fall back to a short-lived file in the # helper's own private tmpdir, which the helper reads and unlinks - # immediately. Egress is POSIX-only (its proxy binds a Unix socket), so - # the Windows config carries no secret — only non-sensitive - # paths/booleans — but we still keep the file private and ephemeral. + # immediately. That tmpdir is created above for every Windows helper — + # including inactive ``sandbox.type: none`` — because the config-file + # path is mandatory on Windows regardless of sandbox activity. Egress + # is POSIX-only (its proxy binds a Unix socket), so the Windows config + # carries no secret — only non-sensitive paths/booleans — but we still + # keep the file private and ephemeral. r_fd: int | None = None if IS_WINDOWS: assert self._tmpdir is not None diff --git a/omnigent/onboarding/harness_install.py b/omnigent/onboarding/harness_install.py index 63cfa9ffe9..584afe0a4e 100644 --- a/omnigent/onboarding/harness_install.py +++ b/omnigent/onboarding/harness_install.py @@ -701,7 +701,8 @@ def harness_cli_logged_in(key: str) -> bool: check=False, timeout=30, capture_output=True, - text=True, + encoding="utf-8", + errors="replace", ) except (OSError, subprocess.TimeoutExpired): return False diff --git a/omnigent/runner/_entry.py b/omnigent/runner/_entry.py index 08f3928b9a..d8406e4175 100644 --- a/omnigent/runner/_entry.py +++ b/omnigent/runner/_entry.py @@ -211,10 +211,12 @@ def auth_flow( ) -> Generator[httpx.Request, httpx.Response, None]: """Inject a fresh ``Authorization`` header before each request. - Fails closed: when the factory is configured but returns no - token (transient SDK failure), raises rather than silently - sending an unauthenticated request. Retries once with a freshly - minted token on either: + User-credential factories (OIDC / Databricks SDK) fail closed when + they return no token. Managed-mint factories that are declined or + temporarily empty send a bare request instead — local single-user + servers need no bearer, and auth-required servers answer with + 401/login redirect so the remint path below can recover. Retries + once with a freshly minted token on either: - HTTP 401 (the standard "your bearer is invalid" response), or - a 3xx redirect whose ``Location`` points at the Databricks @@ -228,9 +230,9 @@ def auth_flow( :param request: The outgoing httpx request. :yields: The request with the auth header set, or - unmodified when no factory is configured. - :raises httpx.RequestError: When the factory is configured - but returns no token. + unmodified when no factory is configured / mint declined. + :raises httpx.RequestError: When a user-credential factory is + configured but returns no token. """ # Workspace routing: name the workspace or the request routes to the # account (the forwarder's POST /events otherwise 403s). Empty when @@ -241,17 +243,24 @@ def auth_flow( request.headers.update(databricks_request_headers(self._server_url)) if self._factory is not None: token = self._factory() - if not token: - if getattr(self._factory, "declined", False): - # The server definitively refuses to mint for this runner - # (managed mint factory hit HTTP 400/404 after install — - # e.g. its construction probe lost a boot race to a - # no-auth server). Bare requests are correct there; do - # NOT fail closed or the runner bricks every callback. - yield request - return - raise httpx.RequestError("Databricks token refresh returned no token") - request.headers["Authorization"] = f"Bearer {token}" + if token: + request.headers["Authorization"] = f"Bearer {token}" + elif getattr(self._factory, "declined", False): + # Server definitively refuses to mint (HTTP 400/404 / Apps + # login redirect). Bare requests are correct for local + # single-user / header-mode servers. + pass + elif isinstance(self._factory, _ManagedMintTokenFactory): + # Mint returned nothing without declining (proxy 5xx, boot + # race 401, transient blip). Local servers accept bare + # requests; auth-required servers answer 401/login redirect + # and the remint path below recovers. + pass + else: + # User-credential factories (OIDC / Databricks SDK) must + # fail closed — a silent bare request would look like a + # mysterious 401 later. + raise httpx.RequestError("server auth token refresh returned no token") response = yield request if self._factory is None: return @@ -676,7 +685,9 @@ def _mint_managed_owner_token( RUNNER_TUNNEL_TOKEN_HEADER: binding_token, **databricks_request_headers(server_url), } - with httpx.Client(timeout=10.0) as client: + # Keep runner-to-server mint requests off system proxies, matching + # the local-server health checks. + with httpx.Client(timeout=10.0, trust_env=False) as client: response = client.post(mint_url, headers=headers) response.raise_for_status() payload = response.json() @@ -1031,6 +1042,9 @@ def create_app( # recorded for this server) routes these callbacks to the workspace. headers={"Origin": OMNIGENT_INTERNAL_WS_ORIGIN, **databricks_request_headers(server_url)}, timeout=httpx.Timeout(5.0, read=None), + # Bypass system proxies so loopback / Apps callbacks hit Omnigent + # directly (see ``_mint_managed_owner_token``). + trust_env=False, # NOTE: ``follow_redirects`` deliberately stays False. # ``_RunnerDatabricksAuth.auth_flow`` needs to *see* the # Databricks Apps OAuth login redirect (302 → diff --git a/omnigent/runner/app.py b/omnigent/runner/app.py index da423f3995..5183fd8b55 100644 --- a/omnigent/runner/app.py +++ b/omnigent/runner/app.py @@ -4116,6 +4116,7 @@ async def _codex_discover_thread_and_forward( headers=headers, auth=_RunnerDatabricksAuth(auth_factory), timeout=httpx.Timeout(10.0), + trust_env=False, ) as _ext_client: _ext_resp = await _ext_client.patch( f"/v1/sessions/{urllib.parse.quote(session_id, safe='')}", @@ -19852,6 +19853,7 @@ def create_runner_app_from_env() -> FastAPI: server_client = httpx.AsyncClient( base_url=server_url, timeout=httpx.Timeout(5.0, read=None), + trust_env=False, ) return create_runner_app(server_client=server_client) diff --git a/omnigent/runtime/workflow.py b/omnigent/runtime/workflow.py index 9b32081e63..0c1de9e76c 100644 --- a/omnigent/runtime/workflow.py +++ b/omnigent/runtime/workflow.py @@ -9,7 +9,6 @@ import json import logging import os -import shlex from dataclasses import dataclass, replace from pathlib import Path from typing import TYPE_CHECKING, Any, Literal @@ -25,6 +24,7 @@ # import annotations`` is in effect). from omnigent.inner.datamodel import OSEnvSpec +from omnigent._platform import static_api_key_print_command from omnigent.entities import ( NON_CONTENT_ITEM_TYPES, CompactionData, @@ -465,20 +465,19 @@ def _provider_auth_command(family: FamilyConfig) -> str: Mirrors the executors' transport contract: the executors' gateway path invokes a shell command that prints the bearer token. A static ``api_key`` (already resolved to plaintext by - :meth:`ProviderEntry.family`) becomes ``printf %s ``; - a user-supplied dynamic ``auth_command`` passes through verbatim. + :meth:`ProviderEntry.family`) becomes a platform-safe print command via + :func:`omnigent._platform.static_api_key_print_command`; a user-supplied + dynamic ``auth_command`` passes through verbatim. :param family: The resolved provider family (``base_url`` + secret expanded by :meth:`ProviderEntry.family`). :returns: A shell command that prints the bearer token to stdout, e.g. - ``"printf %s sk-or-abc"`` or the literal ``auth_command``. + ``"printf %s sk-or-abc"`` on POSIX, or the literal ``auth_command``. :raises OmnigentError: If the family carries neither a static ``api_key`` nor an ``auth_command`` (should not happen post-parse). """ if family.api_key is not None: - # printf %s avoids the trailing newline ``echo`` would add and is - # shell-safe for keys with special characters via shlex.quote. - return f"printf %s {shlex.quote(family.api_key)}" + return static_api_key_print_command(family.api_key) if family.auth_command is not None: return family.auth_command raise OmnigentError( @@ -1205,12 +1204,12 @@ def _build_claude_sdk_spawn_env( # synthesized-provider path above, so no profile or ucode wiring remains # here. The executor strips ANTHROPIC_API_KEY to force subscription auth # inside Claude Code, so the key is threaded via the CLI's apiKeyHelper - # (a shell command the CLI invokes; shlex.quote keeps it shell-safe). + # (a shell command the CLI invokes; platform-safe via static_api_key_print_command). auth_from_spec = spec.executor.auth if auth_from_spec is None: auth_from_spec = _load_global_auth() if isinstance(auth_from_spec, ApiKeyAuth) and auth_from_spec.api_key: - _key_cmd = f"printf %s {shlex.quote(auth_from_spec.api_key)}" + _key_cmd = static_api_key_print_command(auth_from_spec.api_key) env["HARNESS_CLAUDE_SDK_API_KEY_HELPER"] = _key_cmd if auth_from_spec.base_url: env["HARNESS_CLAUDE_SDK_GATEWAY_BASE_URL"] = auth_from_spec.base_url diff --git a/omnigent/spec/_omnigent_compat.py b/omnigent/spec/_omnigent_compat.py index 4986b7cce7..5b1ac4445b 100644 --- a/omnigent/spec/_omnigent_compat.py +++ b/omnigent/spec/_omnigent_compat.py @@ -229,8 +229,8 @@ def is_omnigent_yaml(path: Path) -> bool: if path.suffix.lower() not in {".yaml", ".yml"}: return False try: - raw = yaml.safe_load(path.read_text()) - except yaml.YAMLError: + raw = yaml.safe_load(path.read_text(encoding="utf-8")) + except (yaml.YAMLError, UnicodeDecodeError): return False if not isinstance(raw, dict): return False @@ -269,12 +269,14 @@ def diagnose_yaml_rejection(path: Path) -> str: if path.suffix.lower() not in {".yaml", ".yml"}: return f"file extension is {path.suffix!r}, expected '.yaml' or '.yml'" try: - raw = yaml.safe_load(path.read_text()) + raw = yaml.safe_load(path.read_text(encoding="utf-8")) except yaml.YAMLError as exc: # Strip trailing whitespace so the message stays one line — # PyYAML embeds the source location in its error string, # which is exactly what the user needs to fix the typo. return f"YAML parse error: {exc!s}".replace("\n", " ").rstrip() + except UnicodeDecodeError as exc: + return f"file is not valid UTF-8: {exc}" if raw is None: return "file is empty (or contains only YAML comments / null)" if not isinstance(raw, dict): @@ -377,7 +379,7 @@ def load_omnigent_yaml( # read resolves booleans the same way load_agent_def's YAML # parsing did — both loaders keep on/off as plain strings # instead of the YAML 1.1 bool aliases. - raw = _yaml.load(path.read_text(), Loader=_OmnigentYamlLoader) or {} + raw = _yaml.load(path.read_text(encoding="utf-8"), Loader=_OmnigentYamlLoader) or {} if not isinstance(raw, dict): raw = {} spec = agent_def_to_agent_spec(agent_def, raw_yaml=raw) diff --git a/omnigent/spec/parser.py b/omnigent/spec/parser.py index b5da00f100..0f864a67aa 100644 --- a/omnigent/spec/parser.py +++ b/omnigent/spec/parser.py @@ -182,7 +182,10 @@ def parse(root: Path, *, expand_env: bool = True) -> AgentSpec: if not config_path.exists(): raise FileNotFoundError(f"config.yaml not found in {root}") - raw = yaml.load(config_path.read_text(), Loader=_ConfigYamlLoader) + # Agent bundles are authored as UTF-8 (emoji / em dashes in prompts). + # Path.read_text() defaults to the locale encoding on Windows (often GBK), + # which raises UnicodeDecodeError on those bytes — force UTF-8. + raw = yaml.load(config_path.read_text(encoding="utf-8"), Loader=_ConfigYamlLoader) if not isinstance(raw, dict): raise OmnigentError( f"config.yaml must be a YAML mapping, got {type(raw).__name__}", @@ -1802,7 +1805,7 @@ def _read_contained_file(root: Path, value: str) -> str | None: try: resolved = candidate.resolve() if resolved.is_relative_to(root.resolve()) and resolved.is_file(): - return resolved.read_text() + return resolved.read_text(encoding="utf-8") except OSError: # Path too long or invalid characters — treat as inline text. pass @@ -1843,7 +1846,7 @@ def _resolve_instructions(root: Path, raw_value: object) -> str | None: candidate = root / filename try: if candidate.is_file(): - return candidate.read_text() + return candidate.read_text(encoding="utf-8") except OSError: pass return None @@ -2125,7 +2128,7 @@ def _parse_skill(skill_md: Path) -> SkillSpec: ``strict=False``) can catch them uniformly. """ try: - text = skill_md.read_text() + text = skill_md.read_text(encoding="utf-8") except (OSError, UnicodeDecodeError) as exc: # UnicodeDecodeError (a non-UTF-8 SKILL.md) is a ValueError, not an # OSError — funnel it through OmnigentError too so the lenient @@ -2393,7 +2396,7 @@ def _discover_mcp_servers( return [] servers: list[MCPServerConfig] = [] for yaml_file in sorted(mcp_dir.glob("*.yaml")): - raw = yaml.safe_load(yaml_file.read_text()) + raw = yaml.safe_load(yaml_file.read_text(encoding="utf-8")) if not isinstance(raw, dict): raise OmnigentError( f"MCP config must be a YAML mapping: {yaml_file}", diff --git a/omnigent/spec/skill_sources.py b/omnigent/spec/skill_sources.py index d2fddd4677..079e530f1d 100644 --- a/omnigent/spec/skill_sources.py +++ b/omnigent/spec/skill_sources.py @@ -121,7 +121,7 @@ def resolve_harness_skills(ctx: SkillSourceContext, harness: str | None) -> list def _read_json(path: Path) -> dict[str, Any] | None: """Best-effort JSON read; ``None`` on missing/unreadable/non-dict.""" try: - data = json.loads(path.read_text()) + data = json.loads(path.read_text(encoding="utf-8")) except (OSError, ValueError): return None return data if isinstance(data, dict) else None diff --git a/tests/cli/test_backend.py b/tests/cli/test_backend.py index c6298fe262..20ef7bfd14 100644 --- a/tests/cli/test_backend.py +++ b/tests/cli/test_backend.py @@ -204,6 +204,19 @@ def test_ensure_host_daemon_local_inherits_data_dir_and_db_uri( assert env["OMNIGENT_DATABASE_URI"] == "postgresql://u:pw@h/db" +@pytest.mark.parametrize("server_url", [None, "https://example.databricksapps.com"]) +def test_build_host_daemon_env_forces_utf8_stdio( + monkeypatch: pytest.MonkeyPatch, + server_url: str | None, +) -> None: + """Daemon print output uses the same UTF-8 encoding as its log handler.""" + monkeypatch.setenv("PYTHONIOENCODING", "cp936") + + env = _build_host_daemon_env(server_url=server_url) + + assert env["PYTHONIOENCODING"] == "utf-8:replace" + + def test_build_host_daemon_env_local_preserves_server_credentials( monkeypatch: pytest.MonkeyPatch, ) -> None: diff --git a/tests/host/test_connect.py b/tests/host/test_connect.py index d8ee4f3ed1..28a22e4b16 100644 --- a/tests/host/test_connect.py +++ b/tests/host/test_connect.py @@ -3,8 +3,10 @@ from __future__ import annotations import asyncio +import io import logging import subprocess +import sys import time from pathlib import Path from unittest.mock import patch @@ -14,6 +16,7 @@ from websockets.exceptions import ConnectionClosedError, InvalidStatus, InvalidURI from websockets.http11 import Response +import omnigent.host.connect as connect from omnigent.host.connect import ( HostConnectError, HostProcess, @@ -481,6 +484,54 @@ def _fake_popen(args: list[str], **kwargs: object) -> subprocess.Popen[bytes]: _cleanup_host(host) +async def test_handle_launch_returns_result_when_banner_is_not_gbk_encodable( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A Unicode log path cannot interrupt launch after the runner is tracked.""" + + class _LiveProcess: + pid = 4242 + returncode: int | None = None + + def poll(self) -> int | None: + return self.returncode + + def terminate(self) -> None: + self.returncode = 0 + + def wait(self, timeout: float | None = None) -> int: + return self.returncode or 0 + + def kill(self) -> None: + self.returncode = -9 + + unicode_home = tmp_path / "home-😀" + unicode_home.mkdir() + workspace = tmp_path / "project" + workspace.mkdir() + monkeypatch.setattr(Path, "home", classmethod(lambda _cls: unicode_home)) + output = io.TextIOWrapper(io.BytesIO(), encoding="gbk", errors="strict", write_through=True) + monkeypatch.setattr(sys, "stdout", output) + monkeypatch.setattr( + "omnigent.host.connect.subprocess.Popen", + lambda *args, **kwargs: _LiveProcess(), + ) + host = _make_host_process() + + result = await host._handle_launch( + HostLaunchRunnerFrame( + request_id="req_unicode_log", + binding_token="tok_unicode_log", + workspace=str(workspace), + ) + ) + + assert result.status == "launched" + assert result.runner_id in host._runners + _cleanup_host(host) + + class _FakeTunnel: """In-memory stand-in for the host's WebSocket tunnel connection. @@ -1130,6 +1181,27 @@ def test_reap_orphans_reaps_orphaned_children(tmp_path: Path) -> None: assert host._reap_orphans_once() == 0 +async def test_reap_orphans_is_noop_without_posix_wait_primitives( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Windows has no waitpid/WNOHANG, so its host must skip orphan reaping.""" + host = _make_host_process() + monkeypatch.delattr(connect.os, "waitpid", raising=False) + monkeypatch.delattr(connect.os, "WNOHANG", raising=False) + monkeypatch.setattr( + host, + "_reap_orphans_waitid", + lambda: (_ for _ in ()).throw(AssertionError("waitid reaper called")), + ) + monkeypatch.setattr( + host, + "_reap_orphans_waitpid", + lambda: (_ for _ in ()).throw(AssertionError("waitpid reaper called")), + ) + + assert host._reap_orphans_once() == 0 + + def test_reap_orphans_never_steals_tracked_runner_exit_code(tmp_path: Path) -> None: """The reaper must not consume a tracked runner's exit status (#1782). @@ -2566,6 +2638,22 @@ async def test_login_redirect_prints_warning_to_terminal( assert "omnigent login https://app.example.databricks.com" in err +async def test_login_redirect_warning_survives_gbk_stderr( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The warning glyph cannot turn an authentication retry into a disconnect.""" + stderr = io.TextIOWrapper(io.BytesIO(), encoding="gbk", errors="strict", write_through=True) + monkeypatch.setattr(sys, "stderr", stderr) + host = _host() + + result = host._fatal_upgrade_error( + InvalidURI("https://w/oidc/authorize", "scheme isn't ws or wss") + ) + + assert result is None + assert host._login_redirect_streak == 1 + + async def test_fresh_host_fails_loud_after_persistent_login_redirects( monkeypatch: pytest.MonkeyPatch, ) -> None: diff --git a/tests/host/test_daemon_entry.py b/tests/host/test_daemon_entry.py new file mode 100644 index 0000000000..5d53ef8735 --- /dev/null +++ b/tests/host/test_daemon_entry.py @@ -0,0 +1,24 @@ +"""Tests for the background host daemon process entry point.""" + +from __future__ import annotations + +import sys + +import pytest + +from omnigent import _platform +from omnigent.host import _daemon_entry + + +def test_daemon_configures_stdio_before_argument_validation( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The standalone daemon receives the same Windows stdio hardening as the CLI.""" + calls: list[None] = [] + monkeypatch.setattr(_platform, "configure_cli_stdio", lambda: calls.append(None)) + monkeypatch.setattr(sys, "argv", ["omnigent.host._daemon_entry"]) + + with pytest.raises(SystemExit): + _daemon_entry.main() + + assert calls == [None] diff --git a/tests/inner/test_claude_gateway_shim.py b/tests/inner/test_claude_gateway_shim.py index 0e79ef9b1c..8580d97ca2 100644 --- a/tests/inner/test_claude_gateway_shim.py +++ b/tests/inner/test_claude_gateway_shim.py @@ -341,7 +341,10 @@ async def test_gateway_executor_routes_new_client_through_shim(monkeypatch) -> N ``_get_or_create_client`` (the seam where ``options.env`` is consumed); spawning the real CLI is infeasible in unit tests. """ - from omnigent.inner.claude_sdk_executor import ClaudeSDKExecutor + from omnigent.inner.claude_sdk_executor import ( + ClaudeSDKExecutor, + _cli_settings_for_api_key_helper, + ) from omnigent.inner.databricks_executor import DatabricksCredentials monkeypatch.setattr( @@ -376,8 +379,14 @@ class _StubSDK: from types import SimpleNamespace + upstream_base_url = "https://example.databricks.com/ai-gateway/anthropic" options = SimpleNamespace( - env={"ANTHROPIC_BASE_URL": "https://example.databricks.com/ai-gateway/anthropic"}, + env={"ANTHROPIC_BASE_URL": upstream_base_url}, + settings=_cli_settings_for_api_key_helper( + "databricks auth token --host https://example.databricks.com", + base_url=upstream_base_url, + isolate_user_auth=True, + ), stderr=None, ) try: @@ -393,6 +402,10 @@ class _StubSDK: assert connect_env["ANTHROPIC_BASE_URL"].startswith("http://127.0.0.1:") assert executor._gateway_shim is not None assert connect_env["ANTHROPIC_BASE_URL"] == executor._gateway_shim.base_url + settings = json.loads(options.settings) + assert settings["env"]["ANTHROPIC_BASE_URL"] == executor._gateway_shim.base_url + assert settings["env"]["ANTHROPIC_AUTH_TOKEN"] == "" + assert settings["env"]["ANTHROPIC_API_KEY"] == "" finally: if executor._gateway_shim is not None: await executor._gateway_shim.aclose() diff --git a/tests/inner/test_claude_sdk_executor.py b/tests/inner/test_claude_sdk_executor.py index 3e5c01610a..7f42fad6fd 100644 --- a/tests/inner/test_claude_sdk_executor.py +++ b/tests/inner/test_claude_sdk_executor.py @@ -35,6 +35,17 @@ def _run(coro): loop.close() +def _system_prompt_text(value: object) -> str | None: + """Resolve ClaudeAgentOptions.system_prompt whether inline or spilled to file.""" + if value is None: + return None + if isinstance(value, dict) and value.get("type") == "file": + return Path(value["path"]).read_text(encoding="utf-8") + if isinstance(value, str): + return value + raise AssertionError(f"unexpected system_prompt value: {value!r}") + + # --------------------------------------------------------------------------- # Tests: Prompt extraction # --------------------------------------------------------------------------- @@ -150,6 +161,111 @@ def test_historical_file_data_uri_is_replaced_with_compact_placeholder(self): self.assertIn("What does this document say?", prompt) +# --------------------------------------------------------------------------- +# Tests: Windows CLI argv limit / system-prompt-file spill +# --------------------------------------------------------------------------- + + +class TestSystemPromptForCli(unittest.TestCase): + """Windows CreateProcess argv limit requires --system-prompt-file spill.""" + + def test_oversized_prompt_spills_to_file(self): + from omnigent.inner.claude_sdk_executor import ( + _CLI_INLINE_SYSTEM_PROMPT_MAX_CHARS, + _system_prompt_for_cli, + ) + + body = "P" * (_CLI_INLINE_SYSTEM_PROMPT_MAX_CHARS + 1) + with tempfile.TemporaryDirectory() as tmp: + resolved = _system_prompt_for_cli(body, spill_dir=Path(tmp)) + self.assertIsInstance(resolved, dict) + assert isinstance(resolved, dict) + self.assertEqual(resolved["type"], "file") + path = Path(resolved["path"]) + self.assertTrue(path.is_file()) + self.assertEqual(path.read_text(encoding="utf-8"), body) + + def test_short_prompt_stays_inline_off_windows(self): + from omnigent.inner import claude_sdk_executor as mod + + with patch.object(mod, "IS_WINDOWS", False): + self.assertEqual(mod._system_prompt_for_cli("short prompt"), "short prompt") + + def test_short_prompt_spills_on_windows(self): + from omnigent.inner import claude_sdk_executor as mod + + with ( + patch.object(mod, "IS_WINDOWS", True), + tempfile.TemporaryDirectory() as tmp, + ): + resolved = mod._system_prompt_for_cli("windows short", spill_dir=Path(tmp)) + self.assertIsInstance(resolved, dict) + assert isinstance(resolved, dict) + self.assertEqual(resolved["type"], "file") + self.assertEqual( + Path(resolved["path"]).read_text(encoding="utf-8"), + "windows short", + ) + + def test_polly_sized_prompt_uses_file_shape(self): + """Regression: Polly's ~15KB prompt must not go on Windows argv.""" + import yaml + + from omnigent.inner.claude_sdk_executor import _system_prompt_for_cli + + polly_cfg = Path(__file__).resolve().parents[2] / "examples" / "polly" / "config.yaml" + if not polly_cfg.is_file(): + self.skipTest("examples/polly/config.yaml not present") + prompt = yaml.safe_load(polly_cfg.read_text(encoding="utf-8")).get("prompt") or "" + self.assertGreater(len(prompt), 8000) + with tempfile.TemporaryDirectory() as tmp: + resolved = _system_prompt_for_cli(prompt, spill_dir=Path(tmp)) + self.assertIsInstance(resolved, dict) + assert isinstance(resolved, dict) + self.assertEqual(resolved["type"], "file") + self.assertEqual(Path(resolved["path"]).read_text(encoding="utf-8"), prompt) + + def test_executor_caches_spill_per_session(self): + from omnigent.inner.claude_sdk_executor import ClaudeSDKExecutor + + executor = ClaudeSDKExecutor() + body = "X" * 9000 + first = executor._resolve_cli_system_prompt("sess-1", body) + second = executor._resolve_cli_system_prompt("sess-1", body) + self.assertEqual(first, second) + executor._forget_cli_system_prompt("sess-1") + if isinstance(first, dict): + self.assertFalse(Path(first["path"]).exists()) + + def test_executor_updates_cached_spill_when_prompt_changes(self): + from omnigent.inner.claude_sdk_executor import ClaudeSDKExecutor + + executor = ClaudeSDKExecutor() + first = executor._resolve_cli_system_prompt("sess-1", "A" * 9000) + second = executor._resolve_cli_system_prompt("sess-1", "B" * 9000) + self.assertEqual(first, second) + assert isinstance(second, dict) + self.assertEqual(Path(second["path"]).read_text(encoding="utf-8"), "B" * 9000) + executor._forget_cli_system_prompt("sess-1") + + def test_close_removes_cached_spill_without_live_client(self): + from omnigent.inner.claude_sdk_executor import ClaudeSDKExecutor + + async def _t(): + executor = ClaudeSDKExecutor() + resolved = executor._resolve_cli_system_prompt("sess-1", "X" * 9000) + assert isinstance(resolved, dict) + path = Path(resolved["path"]) + self.assertTrue(path.exists()) + + await executor.close() + + self.assertFalse(path.exists()) + self.assertEqual(executor._cli_system_prompts, {}) + + _run(_t()) + + # --------------------------------------------------------------------------- # Tests: Constructor and properties # --------------------------------------------------------------------------- @@ -1172,6 +1288,14 @@ async def _t(): settings["apiKeyHelper"], "databricks auth token --host https://host", ) + # Higher-precedence --settings env blanks user ~/.claude/settings.json + # auth so a personal coding-plan token cannot 401 the first request. + self.assertEqual(settings["env"]["ANTHROPIC_AUTH_TOKEN"], "") + self.assertEqual(settings["env"]["ANTHROPIC_API_KEY"], "") + self.assertEqual( + settings["env"]["ANTHROPIC_BASE_URL"], + shim_upstream["base_url"], + ) # The CLI talks to the loopback shim; the shim forwards to the # real gateway. A direct gateway URL here would mean the shim was # bypassed and opus thinking.display stays stripped. @@ -1803,7 +1927,7 @@ async def _t(): self.assertIn("mcp__omnigent__sys_session_send", captured_options["allowed_tools"]) self.assertIn( "use `mcp__omnigent__sys_session_send` when instructions say `sys_session_send`", - captured_options["system_prompt"], + _system_prompt_text(captured_options["system_prompt"]), ) self.assertIsInstance(events[-1], TurnComplete) @@ -1898,7 +2022,7 @@ async def _t(): self.assertIn( "use `mcp__omnigent__sys_session_rename` when instructions say " "`sys_session_rename`", - captured_options["system_prompt"], + _system_prompt_text(captured_options["system_prompt"]), ) self.assertIsInstance(events[-1], TurnComplete) @@ -2570,6 +2694,86 @@ async def _t(): # --------------------------------------------------------------------------- +def test_cli_settings_for_managed_gateway_blanks_user_auth_env() -> None: + """``--settings`` must blank AUTH_TOKEN/API_KEY and pin the gateway URL. + + Regression: user ``~/.claude/settings.json`` often carries a personal + coding-plan ``ANTHROPIC_AUTH_TOKEN`` + ``ANTHROPIC_BASE_URL``. Those + settings-file env values outrank the process env, so the first request + after restart 401s against the wrong credential unless Omnigent's + higher-precedence ``--settings`` clears them. + """ + from omnigent.inner.claude_sdk_executor import _cli_settings_for_api_key_helper + + payload = json.loads( + _cli_settings_for_api_key_helper( + "printf %s sk-test", + base_url="https://api.deepseek.com/anthropic", + isolate_user_auth=True, + ) + ) + assert payload["apiKeyHelper"] == "printf %s sk-test" + assert payload["env"]["ANTHROPIC_AUTH_TOKEN"] == "" + assert payload["env"]["ANTHROPIC_API_KEY"] == "" + assert payload["env"]["ANTHROPIC_BASE_URL"] == "https://api.deepseek.com/anthropic" + + +def test_cli_settings_for_helper_only_preserves_user_auth_env() -> None: + """A non-gateway helper must not override Claude Code's user settings.""" + from omnigent.inner.claude_sdk_executor import _cli_settings_for_api_key_helper + + payload = json.loads(_cli_settings_for_api_key_helper("printf %s sk-test")) + + assert payload == {"apiKeyHelper": "printf %s sk-test"} + + +def test_cli_settings_for_managed_gateway_requires_base_url() -> None: + """Managed auth must fail loud instead of falling back to a user URL.""" + from omnigent.inner.claude_sdk_executor import _cli_settings_for_api_key_helper + + with pytest.raises(ValueError, match="requires ANTHROPIC_BASE_URL"): + _cli_settings_for_api_key_helper( + "printf %s sk-test", + isolate_user_auth=True, + ) + + +@pytest.mark.parametrize( + "settings", + [ + None, + "not-json", + "{}", + '{"apiKeyHelper":"printf %s sk-test"}', + ], +) +def test_pin_cli_settings_base_url_rejects_invalid_managed_settings(settings) -> None: + """Gateway shim routing must not silently accept an unsafe settings shape.""" + from omnigent.inner.claude_sdk_executor import _pin_cli_settings_base_url + + options = SimpleNamespace(settings=settings) + with pytest.raises(RuntimeError, match="Claude gateway SDK"): + _pin_cli_settings_base_url(options, "http://127.0.0.1:12345") + + +@pytest.mark.asyncio +async def test_gateway_rejects_invalid_settings_before_starting_shim() -> None: + """Invalid managed settings must fail before allocating a loopback shim.""" + from omnigent.inner.claude_sdk_executor import ClaudeSDKExecutor + + executor = ClaudeSDKExecutor() + executor._gateway = True + options = SimpleNamespace( + env={"ANTHROPIC_BASE_URL": "https://gateway.example/anthropic"}, + settings="{}", + ) + + with pytest.raises(RuntimeError, match="missing apiKeyHelper"): + await executor._route_options_through_gateway_shim(options) + + assert executor._gateway_shim is None + + def test_unset_env_var_removes_and_restores(monkeypatch): """Env var present before ``with`` is absent during, restored after.""" from omnigent.inner.claude_sdk_executor import _unset_env_var @@ -2639,14 +2843,16 @@ def test_non_databricks_model_without_routing_does_not_raise() -> None: @pytest.mark.asyncio -async def test_anthropic_api_key_stripped_during_connect(monkeypatch): - """``_get_or_create_client`` must strip ``ANTHROPIC_API_KEY`` from - ``os.environ`` during the ``connect()`` window so the Claude CLI - uses subscription auth instead of a developer API key. +async def test_managed_gateway_auth_stripped_during_connect(monkeypatch): + """A managed gateway must strip ``ANTHROPIC_API_KEY`` and + ``ANTHROPIC_AUTH_TOKEN`` from ``os.environ`` during the ``connect()`` + window so the Claude CLI uses apiKeyHelper instead of a developer API + key or a parent-shell coding-plan token. This test drives ``_get_or_create_client`` with a stub SDK and captures ``os.environ`` at the moment ``connect()`` is invoked, - ensuring both ``CLAUDECODE`` and ``ANTHROPIC_API_KEY`` are absent. + ensuring ``CLAUDECODE``, ``ANTHROPIC_API_KEY``, and + ``ANTHROPIC_AUTH_TOKEN`` are absent. """ from omnigent.inner.claude_sdk_executor import ClaudeSDKExecutor @@ -2679,9 +2885,20 @@ class _StubSDK: ClaudeSDKClient = _StubClient monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-ant-test-secret") + monkeypatch.setenv("ANTHROPIC_AUTH_TOKEN", "settings-token-secret") monkeypatch.setenv("CLAUDECODE", "parent-value") executor = ClaudeSDKExecutor() + executor._gateway = True + + async def _route_through_gateway_shim(options) -> None: + """The environment-isolation test does not need a live shim.""" + + monkeypatch.setattr( + executor, + "_route_options_through_gateway_shim", + _route_through_gateway_shim, + ) options = SimpleNamespace() await executor._get_or_create_client( _StubSDK, # type: ignore[arg-type] @@ -2690,15 +2907,61 @@ class _StubSDK: model=None, ) - # Both keys must be absent at the moment connect() ran. + # Keys must be absent at the moment connect() ran. assert "ANTHROPIC_API_KEY" not in connect_env, ( "ANTHROPIC_API_KEY leaked into connect() -- subscription auth bypassed" ) + assert "ANTHROPIC_AUTH_TOKEN" not in connect_env, ( + "ANTHROPIC_AUTH_TOKEN leaked into connect() -- coding-plan token wins" + ) assert "CLAUDECODE" not in connect_env, ( "CLAUDECODE leaked into connect() -- nested-session error risk" ) - # Both are restored after _get_or_create_client returns. + # Restored after _get_or_create_client returns. + assert os.environ["ANTHROPIC_API_KEY"] == "sk-ant-test-secret" + assert os.environ["ANTHROPIC_AUTH_TOKEN"] == "settings-token-secret" + assert os.environ["CLAUDECODE"] == "parent-value" + + +@pytest.mark.asyncio +async def test_non_gateway_auth_preserved_during_connect(monkeypatch): + """A normal Claude session must retain its native process authentication.""" + from omnigent.inner.claude_sdk_executor import ClaudeSDKExecutor + + connect_env: dict[str, str] = {} + + class _StubClient: + def __init__(self, options): + self.options = options + self._query = None + self._transport = None + + async def connect(self) -> None: + connect_env.update(os.environ) + + async def disconnect(self) -> None: + return None + + class _StubSDK: + ClaudeSDKClient = _StubClient + + monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-ant-test-secret") + monkeypatch.setenv("ANTHROPIC_AUTH_TOKEN", "native-token-secret") + monkeypatch.setenv("CLAUDECODE", "parent-value") + + executor = ClaudeSDKExecutor() + await executor._get_or_create_client( + _StubSDK, # type: ignore[arg-type] + session_key="native-auth-session", + options=SimpleNamespace(), + model=None, + ) + + assert connect_env["ANTHROPIC_API_KEY"] == "sk-ant-test-secret" + assert connect_env["ANTHROPIC_AUTH_TOKEN"] == "native-token-secret" + assert "CLAUDECODE" not in connect_env assert os.environ["ANTHROPIC_API_KEY"] == "sk-ant-test-secret" + assert os.environ["ANTHROPIC_AUTH_TOKEN"] == "native-token-secret" assert os.environ["CLAUDECODE"] == "parent-value" diff --git a/tests/inner/test_codex_executor.py b/tests/inner/test_codex_executor.py index 3aa3dee9bb..9f5af1160f 100644 --- a/tests/inner/test_codex_executor.py +++ b/tests/inner/test_codex_executor.py @@ -246,10 +246,13 @@ def test_constructor_databricks_flag_with_profile_uses_profile_credentials(self) self.assertNotIn('--profile "test-profile" --force-refresh', auth_override) def test_constructor_databricks_flag_with_host_override_skips_profile_lookup(self): + import tomllib + with ( patch("omnigent.inner.codex_executor._find_codex_cli", return_value="/usr/bin/codex"), patch.dict("os.environ", {}, clear=True), patch("omnigent.inner.codex_executor._databricks_gateway_host") as gateway_host, + patch("omnigent.inner.codex_executor.IS_WINDOWS", True), ): executor = CodexExecutor( gateway=True, @@ -274,6 +277,13 @@ def test_constructor_databricks_flag_with_host_override_skips_profile_lookup(sel "databricks auth token --host" in item for item in executor._codex_config_overrides ) ) + parsed = tomllib.loads("\n".join(executor._codex_config_overrides)) + auth = parsed["model_providers"]["omnigent_databricks"]["auth"] + self.assertEqual(auth["command"], "powershell.exe") + self.assertEqual( + auth["args"], + ["-NoProfile", "-NonInteractive", "-Command", "& printf token"], + ) def test_constructor_databricks_flag_with_host_override_requires_base_url(self): with ( diff --git a/tests/inner/test_loader.py b/tests/inner/test_loader.py index 1cbbc1bb97..cbe30cbdc6 100644 --- a/tests/inner/test_loader.py +++ b/tests/inner/test_loader.py @@ -420,6 +420,16 @@ def test_workflow(self): class TestLoadFromYAML(unittest.TestCase): + def test_load_agent_def_reads_yaml_as_utf8(self): + """UTF-8 punctuation must not depend on the Windows locale codec.""" + with tempfile.TemporaryDirectory() as td: + path = Path(td) / "agent.yaml" + path.write_bytes(b"name: utf8-agent\nprompt: plans \xe2\x80\x94 and splits\n") + + agent = load_agent_def(path) + + self.assertEqual(agent.prompt, "plans — and splits") + def test_yaml_on_key_is_not_parsed_as_boolean(self): yaml_content = """ name: policy_agent diff --git a/tests/inner/test_os_env.py b/tests/inner/test_os_env.py index 1654b560f0..ea6a3491a5 100644 --- a/tests/inner/test_os_env.py +++ b/tests/inner/test_os_env.py @@ -373,6 +373,7 @@ def test_child_shell_env_noop_without_pythonpath( # --------------------------------------------------------------------------- +@pytest.mark.posix_only def test_shell_command_does_not_see_omnigent_project_root( monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -384,6 +385,12 @@ def test_shell_command_does_not_see_omnigent_project_root( sees the sibling project entry but not omnigent's, so project subprocesses resolve their own packages. + POSIX-only: Windows often resolves ``bash`` to WSL's ``bash.exe``, which + does not inherit the Windows process ``PYTHONPATH`` the way a native + POSIX shell does. The strip logic itself is covered by the + ``_child_shell_env`` unit tests above; helper startup on Windows is + covered by :func:`test_inactive_sandbox_helper_starts`. + :returns: None. """ project_entry = "/opt/venvs/proj/site-packages" @@ -401,3 +408,28 @@ def test_shell_command_does_not_see_omnigent_project_root( out = result.get("stdout", "") assert project_entry in out assert str(_project_root()) not in out + + +def test_inactive_sandbox_helper_starts() -> None: + """``sandbox.type: none`` must start the OS helper on every platform. + + Regression for Windows: config is delivered via ``--config-file`` in a + private tmpdir, but that tmpdir used to be created only when + ``sandbox.active``. Inactive policies then hit + ``assert self._tmpdir is not None`` before the helper could spawn, so + ``sys_os_shell`` / ``sys_os_read`` failed immediately. + + :returns: None. + """ + os_env = create_os_environment( + OSEnvSpec(type="caller_process", sandbox=OSEnvSandboxSpec(type="none")) + ) + assert os_env is not None + try: + result = asyncio.run(os_env.shell("echo omnigent-osenv-ok")) + finally: + os_env.close() + + assert "error" not in result, result + assert result.get("exit_code") == 0 + assert "omnigent-osenv-ok" in (result.get("stdout") or "") diff --git a/tests/inner/test_proc_and_platform.py b/tests/inner/test_proc_and_platform.py index b1e5c25b9f..71bccf2463 100644 --- a/tests/inner/test_proc_and_platform.py +++ b/tests/inner/test_proc_and_platform.py @@ -9,8 +9,10 @@ from __future__ import annotations import contextlib +import io import os import subprocess +import sys import time from pathlib import Path @@ -21,6 +23,13 @@ from omnigent.inner import _proc +class _TTYTextIOWrapper(io.TextIOWrapper): + """Text stream stand-in that reports an attached terminal.""" + + def isatty(self) -> bool: + return True + + def _spin_cmd() -> list[str]: """A short-lived child process that does nothing but sleep.""" if os.name == "nt": @@ -40,6 +49,124 @@ def test_platform_flags_are_mutually_consistent() -> None: assert _platform.IS_WINDOWS != _platform.IS_POSIX +def test_static_api_key_print_command_posix(monkeypatch: pytest.MonkeyPatch) -> None: + """POSIX emits ``printf %s`` with a shell-quoted key (no trailing newline).""" + monkeypatch.setattr(_platform, "IS_WINDOWS", False) + assert _platform.static_api_key_print_command("sk-simple") == "printf %s sk-simple" + assert _platform.static_api_key_print_command("sk with spaces") == "printf %s 'sk with spaces'" + + +def test_static_api_key_print_command_windows_prints_exact_bytes( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Windows helper must run under cmd.exe and print the key with no newline. + + Regression for bare ``printf`` apiKeyHelper: cmd.exe has no printf, so Claude + Code fell back to ~/.claude/settings.json tokens and surfaced false 429s. + """ + monkeypatch.setattr(_platform, "IS_WINDOWS", True) + key = "sk-win-test-key-with-$pecial&chars" + cmd = _platform.static_api_key_print_command(key) + assert "printf" not in cmd + # Claude's apiKeyHelper is typically invoked via the process shell (cmd on + # native Windows). Exercise that path, not just a POSIX sh -c. + completed = subprocess.run( + cmd, + shell=True, + check=True, + capture_output=True, + ) + assert completed.stdout == key.encode("utf-8") + assert completed.stderr == b"" + + +def test_static_api_key_print_command_windows_always_quotes_executable( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Shell metacharacters in the Python path must remain inert.""" + monkeypatch.setattr(_platform, "IS_WINDOWS", True) + monkeypatch.setattr(_platform.sys, "executable", r"C:\Tools&More\python.exe") + + cmd = _platform.static_api_key_print_command("sk-test") + + assert cmd.startswith('"C:/Tools&More/python.exe" -c ') + + +def test_configure_cli_stdio_noop_off_windows(monkeypatch: pytest.MonkeyPatch) -> None: + """POSIX (and forced-off Windows) must not touch streams.""" + monkeypatch.setattr(_platform, "IS_WINDOWS", False) + assert _platform.configure_cli_stdio() is False + + +def test_configure_cli_stdio_reconfigures_gbk_console_streams( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Legacy GBK TextIOWrappers become UTF-8 so emoji / ✓ are printable.""" + monkeypatch.setattr(_platform, "IS_WINDOWS", True) + output_cp_calls: list[None] = [] + monkeypatch.setattr( + _platform, + "_set_windows_console_output_utf8", + lambda: output_cp_calls.append(None), + ) + out = _TTYTextIOWrapper(io.BytesIO(), encoding="gbk", errors="strict", write_through=True) + err = _TTYTextIOWrapper(io.BytesIO(), encoding="gbk", errors="strict", write_through=True) + monkeypatch.setattr(sys, "stdout", out) + monkeypatch.setattr(sys, "stderr", err) + # Probe: the pre-fix stream rejects the host banner glyph. + with pytest.raises(UnicodeEncodeError): + out.write("✓") + out.seek(0) + out.truncate() + + assert _platform.configure_cli_stdio() is True + assert out.encoding == "utf-8" + assert err.encoding == "utf-8" + assert output_cp_calls == [None] + # Must not raise — this is the Phase 0 CP936 failure mode. + print("✓ Connected 🖥️", file=out, flush=True) + out.seek(0) + assert "✓" in out.read() + + +def test_configure_cli_stdio_preserves_redirected_stream_encoding( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Pipes stay locale-decodable while replacing unsupported glyphs.""" + monkeypatch.setattr(_platform, "IS_WINDOWS", True) + output_cp_calls: list[None] = [] + monkeypatch.setattr( + _platform, + "_set_windows_console_output_utf8", + lambda: output_cp_calls.append(None), + ) + buf = io.BytesIO() + out = io.TextIOWrapper(buf, encoding="gbk", errors="strict", write_through=True) + monkeypatch.setattr(sys, "stdout", out) + monkeypatch.setattr(sys, "stderr", out) + + assert _platform.configure_cli_stdio() is True + assert out.encoding == "gbk" + assert out.errors == "replace" + assert output_cp_calls == [] + + print("✓ Connected", file=out, flush=True) + assert buf.getvalue().decode("gbk").splitlines() == ["? Connected"] + + +def test_safe_console_print_survives_gbk_strict_stream( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A glyph the stream cannot encode must not raise (host-tunnel safety).""" + buf = io.BytesIO() + gbk = io.TextIOWrapper(buf, encoding="gbk", errors="strict", write_through=True) + monkeypatch.setattr(sys, "stdout", gbk) + _platform.safe_console_print("✓ Connected as 'host'") + gbk.flush() + # Replacement bytes were written; no exception escaped. + assert buf.getvalue() + + def test_default_shell_argv_runs_an_echo() -> None: argv = _platform.default_shell_argv("echo omnigent-shell-ok") out = subprocess.run(argv, capture_output=True, text=True, check=True) diff --git a/tests/onboarding/test_harness_install.py b/tests/onboarding/test_harness_install.py index a2b9c05045..c94fdbc0b9 100644 --- a/tests/onboarding/test_harness_install.py +++ b/tests/onboarding/test_harness_install.py @@ -832,6 +832,28 @@ def _run(argv: list[str], **k: object): assert hi.harness_cli_logged_in(ANTHROPIC_FAMILY) is expected +def test_harness_cli_logged_in_decodes_status_output_as_utf8( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Status output uses the CLI's UTF-8 pipe encoding, not the system locale.""" + monkeypatch.setattr(hi.shutil, "which", lambda name: f"/usr/bin/{name}") + + def _run(argv: list[str], **kwargs: object): + assert argv == ["claude", "auth", "status"] + assert kwargs["encoding"] == "utf-8" + assert kwargs["errors"] == "replace" + assert "text" not in kwargs + return subprocess.CompletedProcess( + args=argv, + returncode=0, + stdout='{"loggedIn": true, "message": "signed in — ✓"}', + stderr="", + ) + + monkeypatch.setattr(hi.subprocess, "run", _run) + assert hi.harness_cli_logged_in(ANTHROPIC_FAMILY) is True + + @pytest.mark.parametrize( "stdout,returncode,expected", [ diff --git a/tests/runner/test_runner_entry.py b/tests/runner/test_runner_entry.py index adc32b57ef..b820783b4e 100644 --- a/tests/runner/test_runner_entry.py +++ b/tests/runner/test_runner_entry.py @@ -25,6 +25,7 @@ _load_runner_idle_timeout_s_from_config, _make_auth_token_factory, _make_managed_mint_factory, + _ManagedMintTokenFactory, _mint_managed_owner_token, _parent_is_orphaned, _parent_process_is_alive, @@ -584,12 +585,14 @@ def test_mint_managed_owner_token_posts_binding_token_and_parses_response( Locks the runner->server contract: POST /v1/runners/{id}/token with the tunnel binding token in ``X-Omnigent-Runner-Tunnel-Token``, - returning ``{"token", "expires_at"}``. + returning ``{"token", "expires_at"}``. Also asserts ``trust_env=False`` + so a Windows system HTTP proxy cannot swallow loopback mint calls + with an empty 502 that never reaches Omnigent. :param monkeypatch: Pytest environment patch fixture. :returns: None. """ - captured: dict[str, str] = {} + captured: dict[str, Any] = {} def _handler(request: httpx.Request) -> httpx.Response: """Capture the outgoing mint request and return a canned token.""" @@ -602,6 +605,7 @@ def _handler(request: httpx.Request) -> httpx.Response: def _fake_client(**kwargs: Any) -> httpx.Client: """Build a real sync client backed by the capturing MockTransport.""" + captured["client_kwargs"] = dict(kwargs) return real_client(transport=httpx.MockTransport(_handler), **kwargs) monkeypatch.setattr("omnigent.runner._entry.httpx.Client", _fake_client) @@ -617,6 +621,95 @@ def _fake_client(**kwargs: Any) -> httpx.Client: assert captured["method"] == "POST" assert captured["binding_token"] == "the-binding-token" assert captured["url"].endswith("/v1/runners/runner_token_abc/token") + assert captured["client_kwargs"].get("trust_env") is False + + +def test_managed_mint_transient_502_sends_bare_request( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A non-declining mint failure must not brick callbacks as Databricks. + + Reproduces the Windows + system-proxy failure mode: mint returns HTTP + 502 (empty body from the proxy) so the factory stays non-declined and + empty. Auth must send a bare request instead of raising the old + ``Databricks token refresh returned no token`` error that killed every + local single-user callback. + + :param monkeypatch: Pytest environment patch fixture. + :returns: None. + """ + + def _proxy_502(mint_url: str, server_url: str, binding_token: str) -> tuple[str, float]: + del server_url, binding_token + request = httpx.Request("POST", mint_url) + raise httpx.HTTPStatusError( + "Bad Gateway", + request=request, + response=httpx.Response(502, request=request), + ) + + monkeypatch.setattr("omnigent.runner._entry._mint_managed_owner_token", _proxy_502) + + factory = _ManagedMintTokenFactory( + "https://s.example.com/v1/runners/runner_token_abc/token", + "https://s.example.com", + "btok", + ) + assert factory() is None + assert factory.declined is False + + auth = _RunnerDatabricksAuth(factory) + request = httpx.Request("GET", "http://localhost:6767/v1/agents/ag_1/download") + sent = next(auth.auth_flow(request)) + assert "Authorization" not in sent.headers + + +def test_runner_auth_raises_clear_error_for_empty_user_credential_factory() -> None: + """OIDC/Databricks factories still fail closed with a neutral message. + + :returns: None. + """ + auth = _RunnerDatabricksAuth(lambda: None) + request = httpx.Request("GET", "http://server/v1/agents") + with pytest.raises(httpx.RequestError, match="server auth token refresh returned no token"): + next(auth.auth_flow(request)) + + +def test_make_auth_token_factory_declines_delegated_mint_on_local_header_mode( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Host ``DELEGATED_AUTH=1`` on a no-mint server ends with factory None. + + Local single-user / header mode returns HTTP 400 from mint. The + construction probe must decline so the runner uses bare requests + instead of installing a forever-empty managed-mint factory. + + :param monkeypatch: Pytest environment patch fixture. + :returns: None. + """ + from omnigent.inner.databricks_executor import DatabricksAuthError + + def _no_sdk(profile: str | None = None) -> tuple[Any, str]: + del profile + raise DatabricksAuthError("no Databricks credentials configured") + + def _refuse_mint(mint_url: str, server_url: str, binding_token: str) -> tuple[str, float]: + del server_url, binding_token + request = httpx.Request("POST", mint_url) + raise httpx.HTTPStatusError( + "unsupported in this auth mode", + request=request, + response=httpx.Response(400, request=request), + ) + + monkeypatch.setenv("RUNNER_SERVER_URL", "http://localhost:6767") + monkeypatch.setenv("OMNIGENT_RUNNER_TUNNEL_BINDING_TOKEN", "local-binding-token") + monkeypatch.setenv("OMNIGENT_RUNNER_DELEGATED_AUTH", "1") + monkeypatch.setattr("omnigent.cli_auth.load_token", lambda _url: None) + monkeypatch.setattr("omnigent.inner.databricks_executor._resolve_databricks_auth", _no_sdk) + monkeypatch.setattr("omnigent.runner._entry._mint_managed_owner_token", _refuse_mint) + + assert _make_auth_token_factory() is None def test_runner_databricks_auth_injects_fresh_token_per_request() -> None: diff --git a/tests/runtime/test_provider_spawn_env.py b/tests/runtime/test_provider_spawn_env.py index f0988e57a2..c84711ddc8 100644 --- a/tests/runtime/test_provider_spawn_env.py +++ b/tests/runtime/test_provider_spawn_env.py @@ -25,6 +25,7 @@ import pytest import yaml as _yaml +from omnigent._platform import static_api_key_print_command from omnigent.runtime.workflow import ( _build_claude_sdk_spawn_env, _build_codex_spawn_env, @@ -211,8 +212,10 @@ def test_claude_sdk_uses_anthropic_global_default(config_home: Path) -> None: assert env["HARNESS_CLAUDE_SDK_GATEWAY_BASE_URL"] == "https://anthropic.example.com/v1" # Host is the origin (scheme://netloc) of the base URL, not the full URL. assert env["HARNESS_CLAUDE_SDK_GATEWAY_HOST"] == "https://anthropic.example.com" - # The static key becomes a printf command carrying the resolved secret. - assert env["HARNESS_CLAUDE_SDK_GATEWAY_AUTH_COMMAND"] == "printf %s sk-ant-secret" + # The static key becomes a platform-safe print command carrying the resolved secret. + assert env["HARNESS_CLAUDE_SDK_GATEWAY_AUTH_COMMAND"] == ( + static_api_key_print_command("sk-ant-secret") + ) # No spec model → the family's models.default supplies the model. assert env["HARNESS_CLAUDE_SDK_MODEL"] == "claude-default-model" @@ -241,7 +244,9 @@ def test_detected_ambient_key_routes_with_no_config( assert env["HARNESS_CLAUDE_SDK_GATEWAY"] == "true" assert env["HARNESS_CLAUDE_SDK_GATEWAY_BASE_URL"] == "https://api.anthropic.com" # The ambient key is carried as the printf auth command (resolved, not leaked as a ref). - assert env["HARNESS_CLAUDE_SDK_GATEWAY_AUTH_COMMAND"] == "printf %s sk-ant-detected" + assert env["HARNESS_CLAUDE_SDK_GATEWAY_AUTH_COMMAND"] == ( + static_api_key_print_command("sk-ant-detected") + ) # No pinned model on the detected entry → the catalog default fills in # (non-empty), rather than leaving the model unset. assert env["HARNESS_CLAUDE_SDK_MODEL"] @@ -289,7 +294,9 @@ def test_codex_uses_openai_global_default(config_home: Path) -> None: assert env["HARNESS_CODEX_GATEWAY"] == "true" assert env["HARNESS_CODEX_GATEWAY_BASE_URL"] == "https://openai.example.com/v1" assert env["HARNESS_CODEX_GATEWAY_HOST"] == "https://openai.example.com" - assert env["HARNESS_CODEX_GATEWAY_AUTH_COMMAND"] == "printf %s sk-oai-secret" + assert env["HARNESS_CODEX_GATEWAY_AUTH_COMMAND"] == ( + static_api_key_print_command("sk-oai-secret") + ) assert env["HARNESS_CODEX_MODEL"] == "gpt-default-model" # Codex defaults to the Responses wire API when the family omits wire_api. assert env["HARNESS_CODEX_WIRE_API"] == "responses" @@ -337,7 +344,9 @@ def test_codex_falls_back_to_first_available_openai_credential( # The fallback credentialed the head — full gateway wiring, same as a default. assert env["HARNESS_CODEX_GATEWAY"] == "true" assert env["HARNESS_CODEX_GATEWAY_BASE_URL"] == "https://openai.example.com/v1" - assert env["HARNESS_CODEX_GATEWAY_AUTH_COMMAND"] == "printf %s sk-oai-secret" + assert env["HARNESS_CODEX_GATEWAY_AUTH_COMMAND"] == ( + static_api_key_print_command("sk-oai-secret") + ) # Resolved per spawn — the user's config is NOT mutated (no default written). assert (config_home / "config.yaml").read_text() == before # The fallback is spawn-only: the readout-style resolver (flag off, the @@ -381,7 +390,9 @@ def test_claude_sdk_falls_back_to_first_available_anthropic_credential( assert env["HARNESS_CLAUDE_SDK_GATEWAY"] == "true" assert env["HARNESS_CLAUDE_SDK_GATEWAY_BASE_URL"] == "https://anthropic.example.com/v1" - assert env["HARNESS_CLAUDE_SDK_GATEWAY_AUTH_COMMAND"] == "printf %s sk-ant-secret" + assert env["HARNESS_CLAUDE_SDK_GATEWAY_AUTH_COMMAND"] == ( + static_api_key_print_command("sk-ant-secret") + ) assert (config_home / "config.yaml").read_text() == before assert _resolve_provider_for_build(spec, harness_type="claude-sdk") is None @@ -471,7 +482,9 @@ def test_pi_uses_anthropic_global_default(config_home: Path) -> None: '{"claude": "https://anthropic.example.com/v1"}' ) assert env["HARNESS_PI_GATEWAY_HOST"] == "https://anthropic.example.com" - assert env["HARNESS_PI_GATEWAY_AUTH_COMMAND"] == "printf %s sk-ant-secret" + assert env["HARNESS_PI_GATEWAY_AUTH_COMMAND"] == ( + static_api_key_print_command("sk-ant-secret") + ) assert env["HARNESS_PI_MODEL"] == "claude-default-model" @@ -512,7 +525,9 @@ def test_named_provider_auth_selects_provider_over_global_default(config_home: P # The NAMED provider, not the default, supplies the endpoint + key. assert env["HARNESS_CLAUDE_SDK_GATEWAY_BASE_URL"] == "https://named.example.com/v1" - assert env["HARNESS_CLAUDE_SDK_GATEWAY_AUTH_COMMAND"] == "printf %s sk-named" + assert env["HARNESS_CLAUDE_SDK_GATEWAY_AUTH_COMMAND"] == ( + static_api_key_print_command("sk-named") + ) assert env["HARNESS_CLAUDE_SDK_MODEL"] == "named-model" @@ -655,7 +670,9 @@ def test_claude_sdk_falls_back_to_catalog_default_model(config_home: Path) -> No # var that may ever appear is the profile, which is absent here # because this is a key provider, not a databricks-kind one. assert env["HARNESS_CLAUDE_SDK_GATEWAY"] == "true" - assert env["HARNESS_CLAUDE_SDK_GATEWAY_AUTH_COMMAND"] == "printf %s sk-ant-secret" + assert env["HARNESS_CLAUDE_SDK_GATEWAY_AUTH_COMMAND"] == ( + static_api_key_print_command("sk-ant-secret") + ) assert not any(k.startswith("HARNESS_CLAUDE_SDK_DATABRICKS") for k in env) @@ -736,7 +753,9 @@ def test_qwen_uses_openai_global_default(config_home: Path) -> None: assert env["HARNESS_QWEN_GATEWAY"] == "true" # The base URL host is the origin of the gateway endpoint assert env["HARNESS_QWEN_GATEWAY_HOST"] == "https://openai.example.com" - assert env["HARNESS_QWEN_GATEWAY_AUTH_COMMAND"] == "printf %s sk-oai-secret" + assert env["HARNESS_QWEN_GATEWAY_AUTH_COMMAND"] == ( + static_api_key_print_command("sk-oai-secret") + ) # Model comes from provider's default_model assert env["HARNESS_QWEN_MODEL"] == "gpt-default-model" @@ -950,7 +969,7 @@ def test_no_provider_api_key_path_unchanged(config_home: Path) -> None: env = _build_claude_sdk_spawn_env(spec, workdir=None) # Existing api_key path emits exactly what it did before. - assert env["HARNESS_CLAUDE_SDK_API_KEY_HELPER"] == "printf %s sk-direct" + assert env["HARNESS_CLAUDE_SDK_API_KEY_HELPER"] == (static_api_key_print_command("sk-direct")) # No provider gateway vars leak in (the provider branch did not fire). assert "HARNESS_CLAUDE_SDK_GATEWAY_BASE_URL" not in env assert "HARNESS_CLAUDE_SDK_GATEWAY_AUTH_COMMAND" not in env diff --git a/tests/spec/test_parser.py b/tests/spec/test_parser.py index 04409525f8..5b9efaf458 100644 --- a/tests/spec/test_parser.py +++ b/tests/spec/test_parser.py @@ -38,6 +38,23 @@ def test_parse_minimal(agent_dir: Path) -> None: assert spec.sub_agents == [] +def test_parse_utf8_em_dash_description(tmp_path: Path) -> None: + """Agent YAML with UTF-8 punctuation must load on Windows locale encodings. + + Polly's config uses U+2014 EM DASH (``—``, bytes ``e2 80 94``). On a CP936 + host, bare ``Path.read_text()`` decodes as GBK and raises UnicodeDecodeError + before YAML parsing — the Web UI surfaces that as + ``failed to load agent spec: 'gbk' codec can't decode…``. + """ + (tmp_path / "config.yaml").write_bytes( + b"spec_version: 1\nname: utf8-agent\ndescription: plans \xe2\x80\x94 and splits\n" + ) + spec = parse(tmp_path) + assert spec.name == "utf8-agent" + assert spec.description is not None + assert "\u2014" in spec.description + + def test_parse_missing_config_yaml(tmp_path: Path) -> None: with pytest.raises(FileNotFoundError, match=r"config.yaml not found"): parse(tmp_path) diff --git a/tests/test_claude_native.py b/tests/test_claude_native.py index 8eafa3c869..7501b42a1d 100644 --- a/tests/test_claude_native.py +++ b/tests/test_claude_native.py @@ -22,6 +22,7 @@ from websockets.frames import Close from omnigent import claude_native +from omnigent._platform import static_api_key_print_command from omnigent._runner_startup import RunnerStartupProgress from omnigent._startup_profile import StartupProfiler from omnigent._terminal_picker_theme import PICKER_ACCENT, PICKER_MUTED @@ -6127,7 +6128,7 @@ def test_provider_config_for_native_claude_key_injects_base_url_and_helper() -> "CLAUDE_CODE_DISABLE_EXPERIMENTAL_BETAS": "1", } # Static key delivered via the apiKeyHelper, never the env (allowlist). - assert cfg.api_key_helper == "printf %s sk-ant-test" + assert cfg.api_key_helper == static_api_key_print_command("sk-ant-test") assert cfg.model == "claude-sonnet-4-6" @@ -6278,7 +6279,7 @@ def test_resolve_native_claude_config_spec_provider_default( cfg = claude_native.resolve_native_claude_config(spec=_no_auth_claude_spec()) assert cfg is not None assert cfg.env["ANTHROPIC_BASE_URL"] == "https://api.anthropic.com" - assert cfg.api_key_helper == "printf %s sk-ant-default" + assert cfg.api_key_helper == static_api_key_print_command("sk-ant-default") def test_resolve_native_claude_config_subscription_uses_cli_login( @@ -6369,7 +6370,7 @@ def test_resolve_native_claude_config_ambient_key( assert cfg is not None assert cfg.env["ANTHROPIC_BASE_URL"] == "https://api.anthropic.com" # Resolved from the env ref, delivered via the helper (no secret in env). - assert cfg.api_key_helper == "printf %s sk-ant-ambient" + assert cfg.api_key_helper == static_api_key_print_command("sk-ant-ambient") def test_resolve_native_claude_config_ambient_prefixed_key( @@ -6386,7 +6387,7 @@ def test_resolve_native_claude_config_ambient_prefixed_key( "ANTHROPIC_BASE_URL": "https://api.anthropic.com", "CLAUDE_CODE_DISABLE_EXPERIMENTAL_BETAS": "1", } - assert cfg.api_key_helper == "printf %s sk-ant-prefixed" + assert cfg.api_key_helper == static_api_key_print_command("sk-ant-prefixed") def test_bedrock_config_auth_command_failure_returns_none() -> None: diff --git a/tests/test_native_codex_provider.py b/tests/test_native_codex_provider.py index 505ed2374a..6f46376ea8 100644 --- a/tests/test_native_codex_provider.py +++ b/tests/test_native_codex_provider.py @@ -10,11 +10,14 @@ from __future__ import annotations +import subprocess from pathlib import Path import pytest +import tomllib import yaml +from omnigent._platform import IS_WINDOWS, static_api_key_print_command from omnigent.codex_native_app_server import resolve_native_codex_launch from omnigent.inner.codex_executor import _provider_codex_config_overrides @@ -77,10 +80,68 @@ def test_provider_codex_overrides_coerce_chat_wire_to_responses() -> None: # chat is coerced to responses; codex >= 0.137 rejects a chat config. assert 'wire_api="responses"' in joined assert 'wire_api="chat"' not in joined - # The token command is embedded as the sh auth command. + # The token command is embedded in the platform shell auth config. assert "printf %s sk-or-test" in joined +@pytest.mark.parametrize( + ("is_windows", "expected_command", "expected_args_prefix"), + [ + (False, "sh", ["-c"]), + ( + True, + "powershell.exe", + ["-NoProfile", "-NonInteractive", "-Command"], + ), + ], +) +def test_provider_codex_overrides_use_platform_auth_shell( + monkeypatch: pytest.MonkeyPatch, + is_windows: bool, + expected_command: str, + expected_args_prefix: list[str], +) -> None: + """The final Codex auth process must exist on the target platform.""" + monkeypatch.setattr("omnigent.inner.codex_executor.IS_WINDOWS", is_windows) + auth_command = static_api_key_print_command("sk-test") + + overrides = _provider_codex_config_overrides( + model="gpt-5.5", + base_url="https://api.openai.com/v1", + auth_command=auth_command, + wire_api="responses", + ) + + parsed = tomllib.loads("\n".join(overrides)) + auth = parsed["model_providers"]["omnigent_provider"]["auth"] + assert auth["command"] == expected_command + expected_command_arg = f"& {auth_command}" if is_windows else auth_command + assert auth["args"] == [*expected_args_prefix, expected_command_arg] + + +@pytest.mark.skipif(not IS_WINDOWS, reason="requires Windows PowerShell") +def test_provider_codex_windows_auth_config_prints_exact_key() -> None: + """Execute the final Codex command/args shape, not only the inner helper.""" + key = "sk-win-test-key-with-$pecial&chars" + overrides = _provider_codex_config_overrides( + model="gpt-5.5", + base_url="https://api.openai.com/v1", + auth_command=static_api_key_print_command(key), + wire_api="responses", + ) + parsed = tomllib.loads("\n".join(overrides)) + auth = parsed["model_providers"]["omnigent_provider"]["auth"] + + completed = subprocess.run( + [auth["command"], *auth["args"]], + check=True, + capture_output=True, + ) + + assert completed.stdout == key.encode("utf-8") + assert completed.stderr == b"" + + def test_provider_codex_overrides_preserve_responses_wire() -> None: """An explicit ``responses`` wire passes through unchanged.""" overrides = _provider_codex_config_overrides( @@ -133,7 +194,9 @@ def test_resolve_native_codex_launch_key_default_routes_via_overrides( assert launch.model == "gpt-5.5" joined = "\n".join(launch.config_overrides) assert 'base_url="https://api.openai.com/v1"' in joined - assert "printf %s sk-oai-default" in joined + # Auth command is shell/JSON-escaped in the override; the trailing payload + # (key on POSIX, base64 on Windows) still appears literally. + assert static_api_key_print_command("sk-oai-default").rsplit(None, 1)[-1] in joined def test_resolve_native_codex_launch_openrouter_coerces_chat_wire(_isolated: Path) -> None: @@ -272,7 +335,7 @@ def test_resolve_native_codex_launch_subscription_no_login_falls_through_to_key( assert launch.model == "gpt-5.5" joined = "\n".join(launch.config_overrides) assert 'base_url="https://api.openai.com/v1"' in joined - assert "printf %s sk-oai-real" in joined + assert static_api_key_print_command("sk-oai-real").rsplit(None, 1)[-1] in joined def test_resolve_native_codex_launch_subscription_no_login_no_alternative_uses_login( @@ -343,7 +406,7 @@ def test_resolve_native_codex_launch_ambient_key_routes( assert launch.profile is None joined = "\n".join(launch.config_overrides) assert 'base_url="https://api.openai.com/v1"' in joined - assert "printf %s sk-oai-ambient" in joined + assert static_api_key_print_command("sk-oai-ambient").rsplit(None, 1)[-1] in joined def test_resolve_native_codex_launch_cli_config_default_pins_provider(