fix lefthook (#17023)

.github/workflows/sep-tests.yml

@@ -134,15 +134,17 @@ jobs:
           if [[ "${GITHUB_EVENT_NAME}" == "pull_request" || "${GITHUB_EVENT_NAME}" == "pull_request_target" ]]; then
             changed_files=$(mktemp)
             trap 'rm -f "$changed_files"' EXIT
-            git diff --name-only ${{ github.event.pull_request.base.sha }}...${{ github.event.pull_request.head.sha }} \
-              | while read -r file; do
+            git diff --name-only -z ${{ github.event.pull_request.base.sha }}...${{ github.event.pull_request.head.sha }} \
+              | while IFS= read -r -d '' file; do
                   if [[ -f "$file" ]]; then
                     printf '%s\0' "$file"
                   fi
                 done > "$changed_files"
             echo "Changed files to run lefthook on:"
             if [[ -s "$changed_files" ]]; then
-              tr '\0' '\n' < "$changed_files" | sed 's/^/  /'
+              while IFS= read -r -d '' file; do
+                printf '  %q\n' "$file"
+              done < "$changed_files"
             else
               echo "  (none — lefthook will be a no-op)"
             fi

.github/workflows/tests.yml

@@ -98,15 +98,17 @@ jobs:
           if [[ "${GITHUB_EVENT_NAME}" == "pull_request" || "${GITHUB_EVENT_NAME}" == "pull_request_target" ]]; then
             changed_files=$(mktemp)
             trap 'rm -f "$changed_files"' EXIT
-            git diff --name-only ${{ github.event.pull_request.base.sha }}...${{ github.event.pull_request.head.sha }} \
-              | while read -r file; do
+            git diff --name-only -z ${{ github.event.pull_request.base.sha }}...${{ github.event.pull_request.head.sha }} \
+              | while IFS= read -r -d '' file; do
                   if [[ -f "$file" ]]; then
                     printf '%s\0' "$file"
                   fi
                 done > "$changed_files"
             echo "Changed files to run lefthook on:"
             if [[ -s "$changed_files" ]]; then
-              tr '\0' '\n' < "$changed_files" | sed 's/^/  /'
+              while IFS= read -r -d '' file; do
+                printf '  %q\n' "$file"
+              done < "$changed_files"
             else
               echo "  (none — lefthook will be a no-op)"
             fi

lefthook.yml

@@ -70,32 +70,46 @@ pre-commit:
         else
           gofmt -w {staged_files}
         fi
-    - name: web-prettier
-      # prettier plugins removed from .prettierrc, so this job needs no node_modules.
-      glob: "web/**/*.{css,less,json,js,jsx,ts,tsx}"
-      stage_fixed: true
-      run: |
-        if [ "${LEFTHOOK_CHECK_ONLY:-}" = "1" ]; then
-          cd web && printf '%s\n' {staged_files} | sed 's|^web/||' | xargs npx prettier --check --ignore-unknown
-        else
-          cd web && printf '%s\n' {staged_files} | sed 's|^web/||' | xargs npx prettier --write --ignore-unknown
-        fi
-    - name: web-eslint
-      # Only web-eslint needs node_modules (for @typescript-eslint/* plugins).
-      # web-prettier has no npm deps, so there is no race on node_modules.
-      glob: "web/**/*.{js,jsx,ts,tsx}"
-      stage_fixed: true
-      run: |
-        if [ ! -f web/node_modules/.package-lock.json ]; then
-          echo '==> web/node_modules missing or incomplete; running npm ci --prefix web'
-          rm -rf web/node_modules
-          npm ci --prefix web --no-audit --no-fund
-        fi
-        if [ "${LEFTHOOK_CHECK_ONLY:-}" = "1" ]; then
-          cd web && printf '%s\n' {staged_files} | sed 's|^web/||' | xargs npx eslint
-        else
-          cd web && printf '%s\n' {staged_files} | sed 's|^web/||' | xargs npx eslint --fix
-        fi
+    - name: web-checks
+      group:
+        piped: true
+        jobs:
+          - name: web-deps
+            glob: "web/**/*.{css,less,json,js,jsx,ts,tsx}"
+            run: |
+              # Ensure web/node_modules includes the tooling needed by the hook jobs.
+              # CI runners don't run `npm install` before lefthook, and `npx` without
+              # local node_modules fetches the latest packages from the registry - which
+              # breaks because the pinned prettier plugins
+              # (prettier-plugin-organize-imports, prettier-plugin-packagejson) aren't
+              # auto-resolved and ESLint 10 requires eslint.config.js.
+              if [ ! -f web/node_modules/.package-lock.json ] \
+                || [ ! -d web/node_modules/prettier-plugin-organize-imports ] \
+                || [ ! -d web/node_modules/@typescript-eslint/eslint-plugin ]; then
+                echo '==> web/node_modules missing required hook dependencies; running npm ci --prefix web --include=dev'
+                rm -rf web/node_modules
+                npm ci --prefix web --include=dev --no-audit --no-fund
+              fi
+          - name: web-prettier
+            glob: "web/**/*.{css,less,json,js,jsx,ts,tsx}"
+            root: "web/"
+            stage_fixed: true
+            run: |
+              if [ "${LEFTHOOK_CHECK_ONLY:-}" = "1" ]; then
+                npx prettier --check --ignore-unknown {staged_files}
+              else
+                npx prettier --write --ignore-unknown {staged_files}
+              fi
+          - name: web-eslint
+            glob: "web/**/*.{js,jsx,ts,tsx}"
+            root: "web/"
+            stage_fixed: true
+            run: |
+              if [ "${LEFTHOOK_CHECK_ONLY:-}" = "1" ]; then
+                npx eslint {staged_files}
+              else
+                npx eslint --fix {staged_files}
+              fi
     - name: check-yaml
       run: python3 tools/hooks/check_files.py yaml
     - name: check-json

test/unit_test/tools/test_check_files.py

@@ -0,0 +1,31 @@
+from pathlib import Path
+from subprocess import CompletedProcess
+
+from tools.hooks import check_files
+
+
+def test_staged_paths_preserve_whitespace(monkeypatch):
+    paths = b"space name.py\0line\nbreak.go\0"
+
+    def run(command, **kwargs):
+        assert command == ["git", "diff", "--cached", "--name-only", "--diff-filter=ACMR", "-z"]
+        assert kwargs == {"check": True, "capture_output": True}
+        return CompletedProcess(command, 0, stdout=paths)
+
+    monkeypatch.setattr(check_files.subprocess, "run", run)
+
+    assert check_files._staged_paths() == [Path("space name.py"), Path("line\nbreak.go")]
+
+
+def test_case_conflicts_preserve_newlines(monkeypatch, capsys):
+    paths = b"dir/A\nfile.py\0dir/a\nfile.py\0"
+
+    def run(command, **kwargs):
+        assert command == ["git", "ls-files", "-z"]
+        assert kwargs == {"check": True, "capture_output": True}
+        return CompletedProcess(command, 0, stdout=paths)
+
+    monkeypatch.setattr(check_files.subprocess, "run", run)
+
+    assert check_files.check_case_conflicts([]) == 1
+    assert "case conflict:" in capsys.readouterr().err

tools/hooks/check_files.py

@@ -3,6 +3,7 @@ from __future__ import annotations
 
 import ast
 import json
+import os
 import re
 import subprocess
 import sys
@@ -23,14 +24,17 @@ def _read_bytes(path: Path) -> bytes:
     return path.read_bytes()
 
 
-def _staged_paths() -> list[Path]:
+def _git_paths(*args: str) -> list[Path]:
     proc = subprocess.run(
-        ["git", "diff", "--cached", "--name-only", "--diff-filter=ACMR"],
+        ["git", *args, "-z"],
         check=True,
         capture_output=True,
-        text=True,
     )
-    return [Path(line) for line in proc.stdout.splitlines() if line]
+    return [Path(os.fsdecode(raw_path)) for raw_path in proc.stdout.split(b"\0") if raw_path]
+
+
+def _staged_paths() -> list[Path]:
+    return _git_paths("diff", "--cached", "--name-only", "--diff-filter=ACMR")
 
 
 def _report(errors: list[str]) -> int:
@@ -147,15 +151,10 @@ def check_symlinks(paths: list[Path], fix: bool = False) -> int:
 
 
 def check_case_conflicts(_: list[Path], fix: bool = False) -> int:
-    proc = subprocess.run(
-        ["git", "ls-files"],
-        check=True,
-        capture_output=True,
-        text=True,
-    )
     seen: dict[str, str] = {}
     errors: list[str] = []
-    for path in proc.stdout.splitlines():
+    for raw_path in _git_paths("ls-files"):
+        path = str(raw_path)
         lowered = path.lower()
         other = seen.get(lowered)
         if other and other != path: