mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-01 12:02:05 +00:00
fix(install): scope Playwright override to too-new apt releases + keep step interruptible
Follow-up on #54032 for #35166: - Gate the PLAYWRIGHT_HOST_PLATFORM_OVERRIDE retry on the host being an apt release newer than Playwright recognizes (Ubuntu >24.04 / Debian >13) via playwright_host_unrecognized(), instead of retrying on ANY install failure. A network/disk/permission failure on a supported host now surfaces unchanged rather than getting a mismatched-glibc build forced onto it. - detect_os() now captures DISTRO_VERSION from os-release. - Fold in the interruptibility fix (was PR #35304, self-closed): wrap the download in 'timeout --foreground -k 10' (probed, with plain-timeout fallback) so a terminal Ctrl+C reaches the child and a wedged download is force-killed after the deadline. - Add behavioral tests that source the helpers and assert the retry fires only on Ubuntu 26.04 / Debian 14, not on supported hosts, non-apt distros, native-success, operator-pinned override, or unsupported arch.
This commit is contained in:
parent
a28fe788a6
commit
c0b4a3438a
2 changed files with 230 additions and 14 deletions
|
|
@ -509,8 +509,13 @@ detect_os() {
|
|||
if [ -f /etc/os-release ]; then
|
||||
. /etc/os-release
|
||||
DISTRO="$ID"
|
||||
# VERSION_ID (e.g. "26.04", "14") lets us tell whether the
|
||||
# apt release is newer than the newest one Playwright's
|
||||
# platform resolver recognizes — the #35166 hang condition.
|
||||
DISTRO_VERSION="${VERSION_ID:-}"
|
||||
else
|
||||
DISTRO="unknown"
|
||||
DISTRO_VERSION=""
|
||||
fi
|
||||
fi
|
||||
;;
|
||||
|
|
@ -1885,12 +1890,45 @@ run_browser_install_with_timeout() {
|
|||
shift
|
||||
|
||||
if command -v timeout >/dev/null 2>&1; then
|
||||
timeout "$timeout_seconds" "$@"
|
||||
# GNU `timeout` runs the command in its own process group, so a terminal
|
||||
# Ctrl+C is delivered to `timeout` but never reaches the child — the
|
||||
# download looks frozen and ignores Ctrl+C (#35166). `--foreground`
|
||||
# keeps the command in the shell's foreground group so Ctrl+C reaches
|
||||
# it; `-k 10` sends SIGKILL 10s after the deadline so a wedged download
|
||||
# can't outlive the timeout. Both flags are GNU-only — probe once and
|
||||
# fall back to plain `timeout` on BusyBox (Alpine), and to direct exec
|
||||
# when `timeout` is absent (stock macOS, where Ctrl+C works natively).
|
||||
if timeout --foreground -k 10 1 true >/dev/null 2>&1; then
|
||||
timeout --foreground -k 10 "$timeout_seconds" "$@"
|
||||
else
|
||||
timeout "$timeout_seconds" "$@"
|
||||
fi
|
||||
else
|
||||
"$@"
|
||||
fi
|
||||
}
|
||||
|
||||
# Return success only when the host is an apt release NEWER than the newest one
|
||||
# Playwright's platform resolver recognizes — the exact condition that makes
|
||||
# `playwright install` hang uninterruptibly (#35166). We scope the override
|
||||
# retry to this case rather than retrying on *any* failure, so a genuine
|
||||
# network/disk/permission failure doesn't get a mismatched-glibc build forced
|
||||
# onto it. Newest Playwright-known apt releases as of this writing: Ubuntu
|
||||
# 24.04, Debian 13. Anything above triggers the fallback; everything Playwright
|
||||
# already handles (and every non-apt distro) does not.
|
||||
playwright_host_unrecognized() {
|
||||
# Compare dotted versions: returns 0 if $1 > $2.
|
||||
_ver_gt() {
|
||||
[ "$1" = "$2" ] && return 1
|
||||
[ "$(printf '%s\n%s\n' "$1" "$2" | sort -V | tail -n1)" = "$1" ]
|
||||
}
|
||||
case "$DISTRO" in
|
||||
ubuntu) _ver_gt "${DISTRO_VERSION:-0}" "24.04" ;;
|
||||
debian) _ver_gt "${DISTRO_VERSION:-0}" "13" ;;
|
||||
*) return 1 ;; # Non-apt or unknown — not the #35166 hang condition.
|
||||
esac
|
||||
}
|
||||
|
||||
# Compute the PLAYWRIGHT_HOST_PLATFORM_OVERRIDE value to retry an install with
|
||||
# when Playwright's platform resolver rejects the host. ubuntu24.04 is the
|
||||
# newest Linux build Playwright has shipped across recent releases and runs on
|
||||
|
|
@ -1912,14 +1950,19 @@ playwright_fallback_platform() {
|
|||
# Debian 14, future distros — see #35166), retry it ONCE with
|
||||
# PLAYWRIGHT_HOST_PLATFORM_OVERRIDE pinned to the newest known build.
|
||||
#
|
||||
# This is deliberately try-native-first, override-only-on-failure rather than a
|
||||
# hardcoded distro/version table: when Playwright already supports the host
|
||||
# (e.g. Ubuntu 26.04 on Playwright >=1.61), the first attempt succeeds and the
|
||||
# override never runs — so we never force a mismatched-glibc build onto a
|
||||
# release Playwright handles correctly (microsoft/playwright#35114). It is also
|
||||
# self-correcting: new distro releases work the moment Playwright adds them,
|
||||
# with no change here. Playwright's maintainers bless this env var as the
|
||||
# supported escape hatch for unrecognized platforms (microsoft/playwright#33434).
|
||||
# The override retry is scoped to the actual hang condition: it fires only when
|
||||
# the host is an apt release NEWER than Playwright recognizes
|
||||
# (playwright_host_unrecognized). On every release Playwright already supports
|
||||
# (Ubuntu <=24.04, Debian <=13) and every non-apt distro, the first attempt is
|
||||
# authoritative and a failure is reported as-is — we never force a
|
||||
# mismatched-glibc build (microsoft/playwright#35114) onto a host Playwright
|
||||
# handles correctly. This is deliberately narrower than a retry-on-any-failure:
|
||||
# a network/disk/permission error on a supported host should surface, not get
|
||||
# papered over with a platform override. Playwright's maintainers bless this
|
||||
# env var as the supported escape hatch for unrecognized platforms
|
||||
# (microsoft/playwright#33434); a hardcoded full distro/version table was
|
||||
# rejected upstream (microsoft/playwright#33432), so we only need the
|
||||
# newest-known floor here.
|
||||
#
|
||||
# An operator-provided PLAYWRIGHT_HOST_PLATFORM_OVERRIDE is always respected:
|
||||
# it is inherited by the first attempt, and the retry is skipped.
|
||||
|
|
@ -1940,14 +1983,21 @@ run_playwright_install() {
|
|||
return 1
|
||||
fi
|
||||
|
||||
# Only retry with an override on the apt releases too new for Playwright to
|
||||
# recognize (the #35166 hang). Any other failure is a real failure and is
|
||||
# surfaced unchanged.
|
||||
if ! playwright_host_unrecognized; then
|
||||
return 1
|
||||
fi
|
||||
|
||||
local fallback
|
||||
fallback="$(playwright_fallback_platform)"
|
||||
if [ -z "$fallback" ]; then
|
||||
return 1 # No usable fallback build for this arch.
|
||||
fi
|
||||
|
||||
log_warn "Playwright didn't recognize this platform — retrying with PLAYWRIGHT_HOST_PLATFORM_OVERRIDE=$fallback"
|
||||
log_info "(common on apt releases newer than Playwright knows, e.g. Ubuntu 26.04 / Debian 14; see #35166)"
|
||||
log_warn "Playwright doesn't recognize ${DISTRO} ${DISTRO_VERSION} yet — retrying with PLAYWRIGHT_HOST_PLATFORM_OVERRIDE=$fallback"
|
||||
log_info "(apt releases newer than Playwright knows hang at this step; see #35166)"
|
||||
PLAYWRIGHT_HOST_PLATFORM_OVERRIDE="$fallback" \
|
||||
run_browser_install_with_timeout "$timeout_seconds" "$@"
|
||||
}
|
||||
|
|
|
|||
|
|
@ -102,14 +102,16 @@ def test_playwright_install_retries_with_platform_override_on_failure() -> None:
|
|||
On apt releases newer than Playwright knows (Ubuntu 26.04, Debian 14, future
|
||||
distros) `playwright install` hangs/fails (#35166). run_playwright_install
|
||||
must retry ONCE with PLAYWRIGHT_HOST_PLATFORM_OVERRIDE pinned to the newest
|
||||
known build — but only on failure (try-native-first, so a host Playwright
|
||||
already supports keeps its native build and avoids the glibc mismatch in
|
||||
microsoft/playwright#35114), and never when the operator pinned the value.
|
||||
known build — but only when the host is one of those too-new apt releases
|
||||
(playwright_host_unrecognized), never on a host Playwright already supports
|
||||
(which would force a glibc mismatch, microsoft/playwright#35114), and never
|
||||
when the operator pinned the value.
|
||||
"""
|
||||
text = INSTALL_SH.read_text()
|
||||
|
||||
assert "run_playwright_install()" in text
|
||||
assert "playwright_fallback_platform()" in text
|
||||
assert "playwright_host_unrecognized()" in text
|
||||
# Fallback target is the newest known build, arch-aware.
|
||||
assert 'echo "ubuntu24.04-x64"' in text
|
||||
assert 'echo "ubuntu24.04-arm64"' in text
|
||||
|
|
@ -117,6 +119,170 @@ def test_playwright_install_retries_with_platform_override_on_failure() -> None:
|
|||
assert 'if run_browser_install_with_timeout "$timeout_seconds" "$@" 2>/dev/null; then' in text
|
||||
# Operator-pinned override is respected (retry skipped).
|
||||
assert 'if [ -n "${PLAYWRIGHT_HOST_PLATFORM_OVERRIDE:-}" ]; then' in text
|
||||
# The retry is gated on the unrecognized-apt-release check, not any failure.
|
||||
assert "if ! playwright_host_unrecognized; then" in text
|
||||
# The retry actually sets the override for the child process.
|
||||
assert 'PLAYWRIGHT_HOST_PLATFORM_OVERRIDE="$fallback" \\' in text
|
||||
|
||||
|
||||
def test_browser_install_timeout_stays_interruptible() -> None:
|
||||
"""The Playwright download must stay Ctrl+C-able and force-kill if wedged.
|
||||
|
||||
GNU `timeout` runs the child in its own process group, so a terminal Ctrl+C
|
||||
reaches `timeout` but never the download — it looks frozen and ignores
|
||||
Ctrl+C (#35166). `--foreground` keeps it in the shell's foreground group;
|
||||
`-k 10` guarantees a SIGKILL after the deadline. Both are GNU-only, so the
|
||||
installer probes support once and falls back to plain `timeout`.
|
||||
"""
|
||||
text = INSTALL_SH.read_text()
|
||||
|
||||
# GNU-flag probe + the guarded invocation must both be present.
|
||||
assert "timeout --foreground -k 10 1 true" in text
|
||||
assert 'timeout --foreground -k 10 "$timeout_seconds" "$@"' in text
|
||||
# Plain-timeout fallback preserved for BusyBox/non-GNU.
|
||||
assert 'timeout "$timeout_seconds" "$@"' in text
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Behavioral tests: source the install.sh helpers in a stubbed shell and assert
|
||||
# the override retry fires ONLY on a too-new apt release (#35166), and not on a
|
||||
# host Playwright already supports.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
import subprocess
|
||||
|
||||
|
||||
def _run_install_fn(distro: str, version: str, *, native_fails: bool,
|
||||
arch: str = "x86_64", operator_override: str = "") -> dict:
|
||||
"""Source the relevant functions from install.sh and drive run_playwright_install.
|
||||
|
||||
Stubs `npx` (the install command) to fail/succeed, `uname -m` for arch, and
|
||||
`log_warn`/`log_info` to no-ops. Returns parsed observations: how many times
|
||||
the install command ran, and the override value seen on each run.
|
||||
"""
|
||||
# Extract the functions we need so we don't execute the whole installer.
|
||||
fn_names = [
|
||||
"run_browser_install_with_timeout",
|
||||
"playwright_host_unrecognized",
|
||||
"playwright_fallback_platform",
|
||||
"run_playwright_install",
|
||||
]
|
||||
src = INSTALL_SH.read_text()
|
||||
import re
|
||||
|
||||
extracted = []
|
||||
for name in fn_names:
|
||||
m = re.search(rf"^{re.escape(name)}\(\) \{{.*?^\}}", src, re.MULTILINE | re.DOTALL)
|
||||
assert m, f"could not extract {name}() from install.sh"
|
||||
extracted.append(m.group(0))
|
||||
body = "\n\n".join(extracted)
|
||||
|
||||
native_rc = 1 if native_fails else 0
|
||||
harness = f"""
|
||||
set -u
|
||||
DISTRO={distro!r}
|
||||
DISTRO_VERSION={version!r}
|
||||
export PLAYWRIGHT_HOST_PLATFORM_OVERRIDE={operator_override!r}
|
||||
[ -z "$PLAYWRIGHT_HOST_PLATFORM_OVERRIDE" ] && unset PLAYWRIGHT_HOST_PLATFORM_OVERRIDE
|
||||
|
||||
log_warn() {{ :; }}
|
||||
log_info() {{ :; }}
|
||||
|
||||
# Stub `uname -m` for arch control without touching the real binary.
|
||||
uname() {{ if [ "$1" = "-m" ]; then echo {arch!r}; else command uname "$@"; fi }}
|
||||
|
||||
# Stub `timeout`: just run the command, ignoring flags/duration. We only care
|
||||
# about how the npx stub behaves, not real timeout semantics here.
|
||||
timeout() {{
|
||||
while [ $# -gt 0 ]; do
|
||||
case "$1" in -*|[0-9]*) shift ;; *) break ;; esac
|
||||
done
|
||||
"$@"
|
||||
}}
|
||||
|
||||
# Stub the install command. Record each invocation + the override in effect.
|
||||
npx() {{
|
||||
echo "RUN override=${{PLAYWRIGHT_HOST_PLATFORM_OVERRIDE:-<none>}}" >>"$RUNLOG"
|
||||
# First run reflects native_fails; the override retry (if any) succeeds.
|
||||
if [ -n "${{PLAYWRIGHT_HOST_PLATFORM_OVERRIDE:-}}" ]; then return 0; fi
|
||||
return {native_rc}
|
||||
}}
|
||||
|
||||
{body}
|
||||
|
||||
run_playwright_install 600 npx playwright install --with-deps chromium
|
||||
echo "FINAL_RC=$?"
|
||||
"""
|
||||
import tempfile, os
|
||||
with tempfile.NamedTemporaryFile("w", suffix=".log", delete=False) as lf:
|
||||
runlog = lf.name
|
||||
try:
|
||||
env = dict(os.environ, RUNLOG=runlog)
|
||||
proc = subprocess.run(["bash", "-c", harness], capture_output=True,
|
||||
text=True, env=env)
|
||||
runs = Path(runlog).read_text().strip().splitlines()
|
||||
final_rc = None
|
||||
for line in proc.stdout.splitlines():
|
||||
if line.startswith("FINAL_RC="):
|
||||
final_rc = int(line.split("=", 1)[1])
|
||||
return {"runs": runs, "final_rc": final_rc, "stderr": proc.stderr}
|
||||
finally:
|
||||
Path(runlog).unlink(missing_ok=True)
|
||||
|
||||
|
||||
def test_override_retry_fires_on_ubuntu_26() -> None:
|
||||
"""Ubuntu 26.04 (too new) → native fails → retry with ubuntu24.04 override."""
|
||||
r = _run_install_fn("ubuntu", "26.04", native_fails=True)
|
||||
assert len(r["runs"]) == 2, r["runs"]
|
||||
assert "override=<none>" in r["runs"][0]
|
||||
assert "override=ubuntu24.04-x64" in r["runs"][1]
|
||||
assert r["final_rc"] == 0
|
||||
|
||||
|
||||
def test_override_retry_does_not_fire_on_supported_ubuntu() -> None:
|
||||
"""Ubuntu 24.04 is recognized by Playwright → a failure is surfaced, no override."""
|
||||
r = _run_install_fn("ubuntu", "24.04", native_fails=True)
|
||||
assert len(r["runs"]) == 1, r["runs"]
|
||||
assert "override=<none>" in r["runs"][0]
|
||||
assert r["final_rc"] == 1
|
||||
|
||||
|
||||
def test_override_retry_does_not_fire_on_fedora() -> None:
|
||||
"""Non-apt distro never triggers the override retry, even on failure."""
|
||||
r = _run_install_fn("fedora", "42", native_fails=True)
|
||||
assert len(r["runs"]) == 1, r["runs"]
|
||||
assert r["final_rc"] == 1
|
||||
|
||||
|
||||
def test_override_retry_fires_on_debian_14() -> None:
|
||||
"""Debian 14 (> 13) is the too-new apt case → retry with override."""
|
||||
r = _run_install_fn("debian", "14", native_fails=True)
|
||||
assert len(r["runs"]) == 2, r["runs"]
|
||||
assert "override=ubuntu24.04-x64" in r["runs"][1]
|
||||
assert r["final_rc"] == 0
|
||||
|
||||
|
||||
def test_no_retry_when_native_succeeds_on_ubuntu_26() -> None:
|
||||
"""Even on Ubuntu 26.04, a successful native install is never retried."""
|
||||
r = _run_install_fn("ubuntu", "26.04", native_fails=False)
|
||||
assert len(r["runs"]) == 1, r["runs"]
|
||||
assert "override=<none>" in r["runs"][0]
|
||||
assert r["final_rc"] == 0
|
||||
|
||||
|
||||
def test_operator_override_respected_no_second_run() -> None:
|
||||
"""An operator-pinned override applies to attempt 1; no second run on failure."""
|
||||
r = _run_install_fn("ubuntu", "26.04", native_fails=True,
|
||||
operator_override="ubuntu22.04-x64")
|
||||
# The override is set, so the npx stub returns 0 on the first run.
|
||||
assert len(r["runs"]) == 1, r["runs"]
|
||||
assert "override=ubuntu22.04-x64" in r["runs"][0]
|
||||
assert r["final_rc"] == 0
|
||||
|
||||
|
||||
def test_override_retry_skipped_on_unsupported_arch() -> None:
|
||||
"""Ubuntu 26.04 on an arch with no Playwright build → no fallback retry."""
|
||||
r = _run_install_fn("ubuntu", "26.04", native_fails=True, arch="riscv64")
|
||||
assert len(r["runs"]) == 1, r["runs"]
|
||||
assert r["final_rc"] == 1
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue