fix(pre-commit): skip the node blocks when dashboard provisioning fails

Splitting bootstrap introduced a failure path the old target could not reach:
`make bootstrap-dashboard` can now fail on its own while the run continues.
It set status=1 and fell through into the dashboard lint and gen:api blocks
anyway, which then ran against a stale or absent node_modules and printed a
second failure ("format with npm run format") on top of the real cause. On a
box whose node is below the dashboard's engines floor, that misleading advice
is the last thing on screen and the actual reason has scrolled away.

Skip both node blocks when provisioning fails and say so once. The Python
block shares nothing with the node toolchain, so it still runs.
This commit is contained in:
Claude 2026-08-08 11:00:53 +00:00
parent 2ee8adf242
commit 42a113a1f3
No known key found for this signature in database
2 changed files with 29 additions and 3 deletions

View file

@ -157,9 +157,14 @@ if [ -n "$e2e_py_files" ]; then
|| { echo "✗ Raw HTTP client import in tests/e2e. Route the call through tests/e2e/e2e_http.py, then re-run make pre-commit." >&2; status=1; }
fi
dashboard_ready=1
if [ -n "$ui_prettier_files" ] || [ -n "$ui_eslint_files" ] || [ -n "$spec_files" ]; then
echo "pre-commit: provisioning the dashboard toolchain (make bootstrap-dashboard)"
make bootstrap-dashboard || status=1
if ! make bootstrap-dashboard; then
echo "✗ make bootstrap-dashboard failed, so the dashboard lint and API-type checks are skipped rather than run against an unprovisioned toolchain. Fix the error above, then re-run make pre-commit." >&2
status=1
dashboard_ready=""
fi
fi
dashboard_checks() {
@ -172,7 +177,7 @@ dashboard_checks() {
lint_dashboard || { echo "✗ Dashboard lint failed. See above; format with: (cd ui/litellm-dashboard && npm run format)." >&2; return 1; }
}
if [ -n "$ui_prettier_files" ] || [ -n "$ui_eslint_files" ]; then
if [ -n "$dashboard_ready" ] && { [ -n "$ui_prettier_files" ] || [ -n "$ui_eslint_files" ]; }; then
dash_log=$(mktemp)
set -m
dashboard_checks > "$dash_log" 2>&1 &
@ -210,7 +215,7 @@ genapi_checks() {
return $status
}
if [ -n "$spec_files" ]; then
if [ -n "$dashboard_ready" ] && [ -n "$spec_files" ]; then
gen_log=$(mktemp)
set -m
genapi_checks > "$gen_log" 2>&1 &

View file

@ -31,6 +31,9 @@ MAKE_STUB = """#!/bin/sh
. "$STUB_BIN/barrier.sh"
[ -n "${STUB_MAKE_LOG:-}" ] && echo "$*" >> "$STUB_MAKE_LOG"
case "$*" in
bootstrap-dashboard)
[ "${STUB_FAIL:-}" = "bootstrap-dashboard" ] && exit 1
;;
lint)
[ "${STUB_FAIL:-}" = "make-lint" ] && exit 1
[ -n "${STUB_BARRIER_DIR:-}" ] && barrier_sync python "dashboard genapi"
@ -175,6 +178,24 @@ def test_the_dashboard_is_provisioned_once_when_both_node_blocks_run(tmp_path: P
assert invocations.count("bootstrap-dashboard") == 1
def test_failed_dashboard_provisioning_skips_the_node_blocks_rather_than_running_them(tmp_path: Path) -> None:
"""A failed `make bootstrap-dashboard` must not fall through into the node blocks.
They would run against a stale or absent node_modules and report a second,
misleading failure ("format with npm run format") on top of the real cause,
which is the one a reader needs. The Python block shares nothing with the node
toolchain, so it still has to run."""
repo, bin_dir = _sandbox(tmp_path)
proc = _run(repo, bin_dir, {"STUB_FAIL": "bootstrap-dashboard"})
combined = proc.stdout + proc.stderr
assert proc.returncode == 1
assert "make bootstrap-dashboard failed" in combined
assert "linting dashboard" not in combined
assert "API types" not in combined
assert "Dashboard lint failed" not in combined
assert "linting Python" in combined
def test_full_output_is_saved_to_a_log_file_in_the_git_dir(tmp_path: Path) -> None:
repo, bin_dir = _sandbox(tmp_path)
(repo / "scratch.txt").write_text("")