fix(install): stop a CLI install from building the desktop's node-pty
The browser-tools step ran a bare `npm install` at the repo root, which resolves the root package.json's `apps/*` workspace glob. That materializes apps/desktop and with it node-pty, which ships no Linux prebuild and falls back to `node-gyp rebuild` — so the installer needs make/gcc on a machine that will never launch Electron or a PTY addon. Since #85297 made a failed npm install fatal, a host without a C toolchain (a stock CentOS/RHEL box, for instance) cannot complete a CLI-only install at all; it just reports "npm install failed or timed out". Name the workspaces the install actually needs instead. ui-tui and web are selected when present, with --include-workspace-root so the root's shared ESLint devDependencies are not pruned by the scoped install — the same closure `hermes update` already installs. A checkout with neither workspace falls back to a root-only install, since npm fails hard on a workspace it cannot find. Desktop dependencies keep coming from install_desktop(), which is only reachable via --include-desktop. Against a pristine tree the unscoped install reifies 1362 packages including node-pty 1.1.0; the scoped one reifies 582 with no native desktop addon. A fork force-push can 404 the compare API used by detect-changes, which fail-opens with ci_review=true and blocks the PR on a ci-reviewed label the install change does not need. Recover the file list from the pull request files endpoint before that fail-open.
This commit is contained in:
@@ -57,6 +57,7 @@ from __future__ import annotations
|
||||
|
||||
import json
|
||||
import os
|
||||
import subprocess
|
||||
import sys
|
||||
|
||||
_FRONTEND = ("ui-tui/", "web/", "apps/") # TS typecheck-matrix packages
|
||||
@@ -230,9 +231,72 @@ def classify(files: list[str]) -> dict[str, bool]:
|
||||
return ret
|
||||
|
||||
|
||||
def _pull_request_number() -> str | None:
|
||||
"""Read the PR number from the Actions event payload, if present."""
|
||||
event_path = os.environ.get("GITHUB_EVENT_PATH")
|
||||
if not event_path:
|
||||
return None
|
||||
try:
|
||||
with open(event_path, encoding="utf-8") as fh:
|
||||
payload = json.load(fh)
|
||||
except (OSError, json.JSONDecodeError):
|
||||
return None
|
||||
number = (payload.get("pull_request") or {}).get("number")
|
||||
return str(number) if number else None
|
||||
|
||||
|
||||
def pull_request_changed_files() -> list[str]:
|
||||
"""Recover the PR file list when the compare API returned nothing.
|
||||
|
||||
``detect-changes`` calls ``repos/.../compare/base...head`` with raw SHAs.
|
||||
A fork force-push can 404 for ~30s until GitHub attaches the new head SHA
|
||||
to the base repo, so the action fails open with an empty file list. That
|
||||
forces ``ci_review=true`` and blocks the PR on a ``ci-reviewed`` label
|
||||
even when no CI-sensitive file changed.
|
||||
|
||||
The pull-request files endpoint already knows the PR's files (it is how
|
||||
this action used to classify), so use it as a fallback on pull_request
|
||||
events only. Push/dispatch keep the empty-diff fail-open.
|
||||
"""
|
||||
if os.environ.get("EVENT_NAME") != "pull_request":
|
||||
return []
|
||||
repo = os.environ.get("REPO") or os.environ.get("GITHUB_REPOSITORY") or ""
|
||||
pr = _pull_request_number()
|
||||
if not repo or not pr:
|
||||
return []
|
||||
try:
|
||||
completed = subprocess.run(
|
||||
[
|
||||
"gh",
|
||||
"api",
|
||||
"--paginate",
|
||||
f"repos/{repo}/pulls/{pr}/files",
|
||||
"--jq",
|
||||
".[].filename",
|
||||
],
|
||||
check=False,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=30,
|
||||
)
|
||||
except (OSError, subprocess.TimeoutExpired):
|
||||
return []
|
||||
if completed.returncode != 0:
|
||||
return []
|
||||
return [line.strip() for line in completed.stdout.splitlines() if line.strip()]
|
||||
|
||||
|
||||
def main() -> int:
|
||||
files = sys.stdin.read().splitlines()
|
||||
if not any(f.strip() for f in files):
|
||||
recovered = pull_request_changed_files()
|
||||
if recovered:
|
||||
print(
|
||||
f"compare API returned no files; recovered {len(recovered)} "
|
||||
"path(s) from the pull request files endpoint",
|
||||
file=sys.stderr,
|
||||
)
|
||||
files = recovered
|
||||
lanes = classify(files)
|
||||
out = "\n".join([
|
||||
*(f"{key}={str(value).lower()}" for key, value in lanes.items()),
|
||||
|
||||
+34
-1
@@ -2531,6 +2531,36 @@ configure_browser_env_from_system_browser() {
|
||||
log_success "Configured browser tools to use $browser_path"
|
||||
}
|
||||
|
||||
# Select the npm workspaces a CLI install actually needs, into the
|
||||
# NODE_DEPS_WORKSPACE_ARGS array.
|
||||
#
|
||||
# A bare `npm install` at the repo root resolves package.json's `apps/*`
|
||||
# glob, which materializes apps/desktop — and with it node-pty, which ships
|
||||
# no Linux prebuild and falls back to `node-gyp rebuild`. On a host without
|
||||
# make/gcc that rebuild fails, and since #85297 made a failed npm install
|
||||
# fatal it aborts the whole install of a machine that will never launch
|
||||
# Electron or a PTY addon (#38311, #38772). Desktop dependencies are
|
||||
# installed by install_desktop(), reachable only via --include-desktop.
|
||||
#
|
||||
# Naming ui-tui/web excludes the unnamed apps/* workspaces, and
|
||||
# --include-workspace-root keeps the root's own devDependencies (the shared
|
||||
# ESLint flat config each workspace imports) from being pruned by the scoped
|
||||
# install — the same closure `hermes update` installs
|
||||
# (hermes_cli/main.py::_update_node_dependencies). Prebuilt/partial checkouts
|
||||
# can lack a workspace, and naming a missing one makes npm fail hard, so fall
|
||||
# back to a root-only install that still skips apps/*.
|
||||
node_deps_workspace_args() {
|
||||
local install_dir="$1"
|
||||
NODE_DEPS_WORKSPACE_ARGS=()
|
||||
[ -f "$install_dir/ui-tui/package.json" ] && NODE_DEPS_WORKSPACE_ARGS+=(--workspace ui-tui)
|
||||
[ -f "$install_dir/web/package.json" ] && NODE_DEPS_WORKSPACE_ARGS+=(--workspace web)
|
||||
if [ "${#NODE_DEPS_WORKSPACE_ARGS[@]}" -eq 0 ]; then
|
||||
NODE_DEPS_WORKSPACE_ARGS=(--workspaces=false)
|
||||
return 0
|
||||
fi
|
||||
NODE_DEPS_WORKSPACE_ARGS+=(--include-workspace-root)
|
||||
}
|
||||
|
||||
install_node_deps() {
|
||||
if [ "$HAS_NODE" = false ]; then
|
||||
log_info "Skipping Node.js dependencies (Node not installed)"
|
||||
@@ -2553,9 +2583,12 @@ install_node_deps() {
|
||||
# installed", hiding the degradation from the user (#77003). Now it
|
||||
# fails the install outright instead of burying the warning (#85297).
|
||||
# Capture npm output so failures are diagnosable (#87340).
|
||||
# Scoped to the workspaces a CLI install needs so apps/desktop's
|
||||
# node-pty is never built here — see node_deps_workspace_args().
|
||||
node_deps_workspace_args "$INSTALL_DIR"
|
||||
local npm_log
|
||||
npm_log="$(mktemp)"
|
||||
if ! run_with_timeout "$NODE_DEPS_TIMEOUT" npm install --silent \
|
||||
if ! run_with_timeout "$NODE_DEPS_TIMEOUT" npm install "${NODE_DEPS_WORKSPACE_ARGS[@]}" --silent \
|
||||
>"$npm_log" 2>&1; then
|
||||
log_error "npm install failed or timed out; Node.js dependencies were not installed"
|
||||
if [ -s "$npm_log" ]; then
|
||||
|
||||
Reference in New Issue
Block a user