From 42a113a1f3816e199e9c0b95e7316a4f187bf1e0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 11:00:53 +0000 Subject: [PATCH] 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. --- scripts/pre_commit_lint.sh | 11 ++++++++--- tests/test_litellm/test_pre_commit_lint.py | 21 +++++++++++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/scripts/pre_commit_lint.sh b/scripts/pre_commit_lint.sh index f8ddb528729..fbe409e29ee 100755 --- a/scripts/pre_commit_lint.sh +++ b/scripts/pre_commit_lint.sh @@ -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 & diff --git a/tests/test_litellm/test_pre_commit_lint.py b/tests/test_litellm/test_pre_commit_lint.py index 7efbc8c83be..f3f7ab51465 100644 --- a/tests/test_litellm/test_pre_commit_lint.py +++ b/tests/test_litellm/test_pre_commit_lint.py @@ -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("")