mirror of
https://github.com/open-jarvis/OpenJarvis.git
synced 2026-07-27 21:05:34 +00:00
fix(windows): cross-platform python discovery + browser open helpers (#436)
Reimplements the useful parts of #385 cleanly. Adds two small cross-platform helpers under `openjarvis.core.utils`: - `get_python_executable()` — prefers `python3`, falls back to `python` for Windows / minimal distros that only ship the unversioned name. - `open_browser(url)` — `webbrowser.open` by default; on Windows uses `cmd /c start "" <url>` to avoid console-host edge cases. Swapped at every hardcoded `python3` / `webbrowser.open` site: `connectors/oauth.py`, `evals/scorers/livecodebench.py`, `scripts/oauth_all.py`, `scripts/install/install.sh` (adds `PY_CMD` detection block), `scripts/quickstart.sh` (adds `MINGW*|MSYS*|CYGWIN*) cmd /c start` case), and the two affected test files. Test files wrap `get_python_executable()` in `shlex.quote()` before interpolating into `shell=True` strings — Windows interpreter paths often contain spaces. Deliberately different from #385: `openjarvis.core.__init__` does NOT re-export `DEFAULT_CONFIG_DIR` (would have raised ImportError because it's in `openjarvis.core.config`, not the package `__init__`; re-exporting would also force eager import of the heavy config module at every `import openjarvis.core`). `oauth.py` keeps `from openjarvis.core.config import DEFAULT_CONFIG_DIR` alongside the new `from openjarvis.core import open_browser`. Original API surface and call-site sweep by @sanjayravit in #385 — huge thanks for the careful Windows-compatibility audit. This PR preserves your design while fixing the ImportError edge cases caught during review. Co-Authored-By: sanjayravit <sanjayravit@users.noreply.github.com> Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
co-authored by
sanjayravit
Claude Opus 4.7
parent
ad7c86495f
commit
caa3adbbce
@@ -90,6 +90,18 @@ EOF
|
||||
need git
|
||||
need curl
|
||||
|
||||
# ---- python command ----
|
||||
# Prefer `python3` (Linux / macOS / WSL convention); fall back to `python`
|
||||
# on minimal distros that only ship the unversioned name. This is used by
|
||||
# the analytics beacon and state-file helpers below — uv installs its own
|
||||
# interpreter into the venv later, so this is only for the bootstrap shims.
|
||||
PY_CMD="python3"
|
||||
if ! command -v python3 >/dev/null 2>&1; then
|
||||
if command -v python >/dev/null 2>&1; then
|
||||
PY_CMD="python"
|
||||
fi
|
||||
fi
|
||||
|
||||
# ---- env ----
|
||||
OPENJARVIS_HOME="${OPENJARVIS_HOME:-$HOME/.openjarvis}"
|
||||
OPENJARVIS_REPO_URL="${OPENJARVIS_REPO_URL:-https://github.com/open-jarvis/OpenJarvis.git}"
|
||||
@@ -148,7 +160,7 @@ get_anon_id() {
|
||||
return
|
||||
fi
|
||||
local new_id
|
||||
new_id="$(python3 -c 'import uuid; print(uuid.uuid4())' 2>/dev/null || echo "")"
|
||||
new_id="$("$PY_CMD" -c 'import uuid; print(uuid.uuid4())' 2>/dev/null || echo "")"
|
||||
if [[ -z "$new_id" ]]; then
|
||||
return
|
||||
fi
|
||||
@@ -189,7 +201,7 @@ beacon() {
|
||||
os="$(detect_os)"
|
||||
arch="$(detect_arch)"
|
||||
|
||||
python3 - "$ANALYTICS_HOST" "$ANALYTICS_KEY" "$event" "$anon_id" \
|
||||
"$PY_CMD" - "$ANALYTICS_HOST" "$ANALYTICS_KEY" "$event" "$anon_id" \
|
||||
"$os" "$arch" "$stage" "$elapsed_ms" "$exit_code" \
|
||||
>/dev/null 2>&1 <<'PYEOF' || true
|
||||
import json
|
||||
@@ -251,7 +263,7 @@ mark_done() {
|
||||
if [[ ! -f "$STATE_FILE" ]]; then
|
||||
echo '{}' > "$STATE_FILE"
|
||||
fi
|
||||
python3 - "$STATE_FILE" "$1" "$WSL" <<'PYEOF'
|
||||
"$PY_CMD" - "$STATE_FILE" "$1" "$WSL" <<'PYEOF'
|
||||
import json, sys
|
||||
path, key, wsl = sys.argv[1], sys.argv[2], sys.argv[3]
|
||||
with open(path) as f:
|
||||
|
||||
@@ -11,7 +11,6 @@ from __future__ import annotations
|
||||
import argparse
|
||||
import json
|
||||
import os
|
||||
import webbrowser
|
||||
from http.server import BaseHTTPRequestHandler, HTTPServer
|
||||
from pathlib import Path
|
||||
from typing import Any, Dict, List
|
||||
@@ -19,6 +18,8 @@ from urllib.parse import parse_qs, urlencode, urlparse
|
||||
|
||||
import httpx
|
||||
|
||||
from openjarvis.core import open_browser
|
||||
|
||||
CONFIG_DIR = Path.home() / ".openjarvis" / "connectors"
|
||||
CONFIG_DIR.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
@@ -144,7 +145,7 @@ def do_google() -> None:
|
||||
})
|
||||
)
|
||||
print(" Opening browser...")
|
||||
webbrowser.open(url)
|
||||
open_browser(url)
|
||||
code = _wait_for_code()
|
||||
|
||||
resp = httpx.post(
|
||||
@@ -192,7 +193,7 @@ def do_strava() -> None:
|
||||
})
|
||||
)
|
||||
print(" Opening browser...")
|
||||
webbrowser.open(url)
|
||||
open_browser(url)
|
||||
code = _wait_for_code()
|
||||
|
||||
resp = httpx.post(
|
||||
@@ -303,7 +304,7 @@ def do_spotify() -> None:
|
||||
})
|
||||
)
|
||||
print(" Opening browser...")
|
||||
webbrowser.open(url)
|
||||
open_browser(url)
|
||||
code = _wait_for_code(port=spotify_port)
|
||||
|
||||
import base64
|
||||
|
||||
+14
-8
@@ -47,19 +47,24 @@ echo " └───────────────────────
|
||||
echo -e "${NC}"
|
||||
|
||||
# ── 1. Check Python ──────────────────────────────────────────────────
|
||||
# Prefer python3, fall back to python (Windows / minimal distros that ship
|
||||
# only the unversioned name).
|
||||
info "Checking Python..."
|
||||
if command -v python3 &>/dev/null; then
|
||||
PY_VERSION=$(python3 -c "import sys; print(f'{sys.version_info.major}.{sys.version_info.minor}')")
|
||||
PY_MAJOR=$(echo "$PY_VERSION" | cut -d. -f1)
|
||||
PY_MINOR=$(echo "$PY_VERSION" | cut -d. -f2)
|
||||
if [ "$PY_MAJOR" -ge 3 ] && [ "$PY_MINOR" -ge 10 ]; then
|
||||
ok "Python $PY_VERSION"
|
||||
else
|
||||
fail "Python 3.10+ required (found $PY_VERSION)"
|
||||
fi
|
||||
PY_CMD="python3"
|
||||
elif command -v python &>/dev/null; then
|
||||
PY_CMD="python"
|
||||
else
|
||||
fail "Python 3 not found. Install from https://python.org"
|
||||
fi
|
||||
PY_VERSION=$("$PY_CMD" -c "import sys; print(f'{sys.version_info.major}.{sys.version_info.minor}')")
|
||||
PY_MAJOR=$(echo "$PY_VERSION" | cut -d. -f1)
|
||||
PY_MINOR=$(echo "$PY_VERSION" | cut -d. -f2)
|
||||
if [ "$PY_MAJOR" -ge 3 ] && [ "$PY_MINOR" -ge 10 ]; then
|
||||
ok "Python $PY_VERSION ($PY_CMD)"
|
||||
else
|
||||
fail "Python 3.10+ required (found $PY_VERSION)"
|
||||
fi
|
||||
|
||||
# ── 2. Check / install uv ───────────────────────────────────────────
|
||||
info "Checking uv..."
|
||||
@@ -182,6 +187,7 @@ info "Opening $URL ..."
|
||||
case "$(uname -s)" in
|
||||
Darwin) open "$URL" ;;
|
||||
Linux) xdg-open "$URL" 2>/dev/null || true ;;
|
||||
MINGW*|MSYS*|CYGWIN*) cmd /c start "" "$URL" 2>/dev/null || true ;;
|
||||
*) true ;;
|
||||
esac
|
||||
|
||||
|
||||
@@ -16,6 +16,7 @@ from pathlib import Path
|
||||
from typing import Any, Dict, List, Optional, Tuple
|
||||
from urllib.parse import urlencode
|
||||
|
||||
from openjarvis.core import open_browser
|
||||
from openjarvis.core.config import DEFAULT_CONFIG_DIR
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -419,7 +420,6 @@ def run_oauth_flow(
|
||||
RuntimeError
|
||||
If the user denies authorization or the callback times out.
|
||||
"""
|
||||
import webbrowser
|
||||
from http.server import BaseHTTPRequestHandler, HTTPServer
|
||||
from urllib.parse import parse_qs, urlparse
|
||||
|
||||
@@ -492,7 +492,7 @@ def run_oauth_flow(
|
||||
server.timeout = 120 # 2 minute timeout
|
||||
|
||||
# Open the consent page in the user's default browser
|
||||
webbrowser.open(auth_url)
|
||||
open_browser(auth_url)
|
||||
|
||||
# Wait for the callback (blocking, with per-request timeout)
|
||||
while not auth_code and not error:
|
||||
@@ -661,7 +661,6 @@ def run_connector_oauth(
|
||||
|
||||
Returns the raw token response dict.
|
||||
"""
|
||||
import webbrowser
|
||||
|
||||
provider = get_provider_for_connector(connector_id)
|
||||
if provider is None:
|
||||
@@ -694,7 +693,7 @@ def run_connector_oauth(
|
||||
auth_url = f"{provider.auth_endpoint}?{urlencode(params)}"
|
||||
|
||||
# Open browser and wait for callback
|
||||
webbrowser.open(auth_url)
|
||||
open_browser(auth_url)
|
||||
code = _wait_for_callback_code(
|
||||
host=provider.callback_host,
|
||||
port=provider.callback_port,
|
||||
|
||||
@@ -19,6 +19,7 @@ from openjarvis.core.types import (
|
||||
ToolCall,
|
||||
ToolResult,
|
||||
)
|
||||
from openjarvis.core.utils import get_python_executable, open_browser
|
||||
|
||||
__all__ = [
|
||||
"AgentRegistry",
|
||||
@@ -34,4 +35,6 @@ __all__ = [
|
||||
"ToolCall",
|
||||
"ToolRegistry",
|
||||
"ToolResult",
|
||||
"get_python_executable",
|
||||
"open_browser",
|
||||
]
|
||||
|
||||
@@ -0,0 +1,52 @@
|
||||
"""Small cross-platform utilities used by the CLI, OAuth flow, and evals.
|
||||
|
||||
Kept dependency-free so importing this module is cheap (the public re-export
|
||||
from ``openjarvis.core`` must not pull in heavy modules at package init).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import platform
|
||||
import shutil
|
||||
import subprocess
|
||||
import webbrowser
|
||||
|
||||
|
||||
def get_python_executable() -> str:
|
||||
"""Return the best ``python`` interpreter name on PATH.
|
||||
|
||||
Prefers ``python3`` (Linux/macOS convention); falls back to ``python``
|
||||
(Windows / some minimal Linux distros that ship only ``python``). Returns
|
||||
the literal string ``"python3"`` when neither is found, so callers still
|
||||
get a usable command that will fail with a clear "command not found"
|
||||
rather than an empty string.
|
||||
|
||||
The result is a *command name or absolute path* that callers can hand to
|
||||
:mod:`subprocess` directly when ``shell=False``, and must be shell-quoted
|
||||
(:func:`shlex.quote`) before being interpolated into a ``shell=True``
|
||||
command string — paths on Windows often contain spaces.
|
||||
"""
|
||||
return shutil.which("python3") or shutil.which("python") or "python3"
|
||||
|
||||
|
||||
def open_browser(url: str) -> None:
|
||||
"""Open *url* in the user's default browser, with a Windows fast-path.
|
||||
|
||||
:func:`webbrowser.open` is the cross-platform default, but on Windows it
|
||||
sometimes blocks or fails inside a console host. ``cmd /c start "" "URL"``
|
||||
is the canonical Windows incantation that hands the URL to the OS shell
|
||||
and returns immediately. We try that first on Windows and fall back to
|
||||
:func:`webbrowser.open` if the subprocess spawn fails.
|
||||
"""
|
||||
if platform.system() == "Windows":
|
||||
try:
|
||||
# The empty title argument after ``start`` is required: ``start``
|
||||
# treats a single quoted argument as a window title, not a URL.
|
||||
subprocess.run(["cmd", "/c", "start", "", url], check=False)
|
||||
return
|
||||
except Exception: # noqa: BLE001 - any spawn failure -> fall back
|
||||
pass
|
||||
webbrowser.open(url)
|
||||
|
||||
|
||||
__all__ = ["get_python_executable", "open_browser"]
|
||||
@@ -16,6 +16,7 @@ import subprocess
|
||||
import tempfile
|
||||
from typing import Any, Dict, List, Optional, Tuple
|
||||
|
||||
from openjarvis.core import get_python_executable
|
||||
from openjarvis.evals.core.scorer import Scorer
|
||||
from openjarvis.evals.core.types import EvalRecord
|
||||
|
||||
@@ -90,7 +91,7 @@ def _run_single_test(
|
||||
|
||||
try:
|
||||
result = subprocess.run(
|
||||
["python3", script_path],
|
||||
[get_python_executable(), script_path],
|
||||
input=test_input,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
|
||||
@@ -3,8 +3,10 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import shlex
|
||||
import tempfile
|
||||
|
||||
from openjarvis.core import get_python_executable
|
||||
from openjarvis.security.subprocess_sandbox import (
|
||||
build_safe_env,
|
||||
kill_process_tree,
|
||||
@@ -97,7 +99,7 @@ class TestRunSandboxed:
|
||||
def test_output_truncation(self) -> None:
|
||||
# Generate output larger than max_output_bytes
|
||||
result = run_sandboxed(
|
||||
"python3 -c \"print('A' * 200)\"",
|
||||
f"{shlex.quote(get_python_executable())} -c \"print('A' * 200)\"",
|
||||
timeout=10.0,
|
||||
max_output_bytes=50,
|
||||
)
|
||||
|
||||
@@ -9,11 +9,13 @@ from __future__ import annotations
|
||||
|
||||
import importlib
|
||||
import os
|
||||
import shlex
|
||||
import sys
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from openjarvis.core import get_python_executable
|
||||
from openjarvis.tools.shell_exec import ShellExecTool
|
||||
|
||||
|
||||
@@ -216,7 +218,10 @@ class TestShellExecTool:
|
||||
"""Stdout exceeding 100 KB is truncated."""
|
||||
tool = ShellExecTool()
|
||||
result = tool.execute(
|
||||
command="python3 -c \"print('A' * 200000)\"",
|
||||
command=(
|
||||
f"{shlex.quote(get_python_executable())} "
|
||||
"-c \"print('A' * 200000)\""
|
||||
),
|
||||
)
|
||||
assert "truncated" in result.content
|
||||
assert len(result.content) < 200_000
|
||||
|
||||
Reference in New Issue
Block a user