From 2cb7b5505bd2706a0c56f3c437ac6f9662b71b41 Mon Sep 17 00:00:00 2001
From: atlas <atlas@hyperhive.local>
Date: Wed, 16 Sep 2026 18:20:01 +0200
Subject: [PATCH] scripts: stop collapsing a failed lint scan into a clean pass
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

check-attribution-trailers.sh used `|| true` on `git log`'s exit status,
so a hard failure (bad range, unborn HEAD) and an empty-but-successful
range were indistinguishable — both fell through to the same
`-z "$commits"` exit-0 path. Capture the status via the `if` guard
(exempt from set -e on purpose) and exit 1 on a real git log failure.

check-issue-refs.sh piped `git ls-files | xargs grep | grep -v
lint:allow`, then swallowed the final exit code with `|| true`. Worse:
xargs itself collapses grep's exit 1 (no match) and exit 2+ (real error,
e.g. an unreadable file) into the same xargs(1) status (123 either way),
so even capturing that status can't tell them apart. Switched to `git
grep`, which runs once over the tracked set and hands back its own exit
status untouched (0 matched / 1 no match / 2+ error) — then branch on
that status explicitly for both the scan and the lint:allow filter step.

Refs #4439, #4442
---
 scripts/check-attribution-trailers.sh | 11 +++++-
 scripts/check-issue-refs.sh           | 49 +++++++++++++++++++++------
 2 files changed, 49 insertions(+), 11 deletions(-)

diff --git a/scripts/check-attribution-trailers.sh b/scripts/check-attribution-trailers.sh
index 82cd63dc..9f5032f9 100755
--- a/scripts/check-attribution-trailers.sh
+++ b/scripts/check-attribution-trailers.sh
@@ -35,7 +35,16 @@ if ! git rev-parse --verify "$base" >/dev/null 2>&1; then
   fi
 fi
 
-commits="$(git log --reverse --format='%H' "${base}..HEAD" 2>/dev/null || true)"
+# `|| true` here used to erase git log's own exit status, so a genuine
+# failure (bad range, corrupt ref) and a merely-empty range read the same:
+# both fell through to `-z "$commits"` and exited 0 "clean". Capture the
+# status via the `if` guard instead — that's exempt from `set -e` on
+# purpose — so a failure exits loudly and an empty-but-successful range
+# still means "no commits, nothing to check".
+if ! commits="$(git log --reverse --format='%H' "${base}..HEAD")"; then
+  echo "check-attribution-trailers: git log failed for range ${base}..HEAD — see error above" >&2
+  exit 1
+fi
 
 if [ -z "$commits" ]; then
   exit 0
diff --git a/scripts/check-issue-refs.sh b/scripts/check-issue-refs.sh
index 943906b2..cbca969d 100755
--- a/scripts/check-issue-refs.sh
+++ b/scripts/check-issue-refs.sh
@@ -38,16 +38,45 @@ if [ "$file_count" -eq 0 ]; then
   exit 1
 fi
 
-# `/dev/null` forces grep to always print a filename prefix, even when
-# xargs hands it a single file. `-r`/`-0` keep it robust to odd paths and
-# an empty file list. Lines carrying the `lint:allow` marker are dropped
-# (legitimate non-tracker hit; see the header).
-hits="$(
-  git ls-files -z '*.rs' '*.nix' '*.js' '*.mjs' '*.ts' '*.tsx' '*.css' '*.html' '*.md' \
-    '*.yml' '*.yaml' \
-    | xargs -0 -r grep -nE "$pattern" /dev/null 2>/dev/null \
-    | grep -v 'lint:allow' || true
-)"
+# git grep, not `git ls-files | xargs grep`: grep's exit code is the
+# signal that distinguishes "no tracker refs" (1, clean) from a real
+# scan failure (2+), but piping through xargs collapsed both into
+# xargs(1)'s own 123 either way — indistinguishable, so a real failure
+# (e.g. a file grep can't read) silently read as a clean tree. git grep
+# runs once over the tracked set matching these globs and hands back its
+# own exit status untouched: 0 = matched, 1 = no match, 2+/128 = error.
+#
+# The `if` guard (not `|| true`) is what's exempt from `set -e` here —
+# capture the status, then branch on it explicitly rather than erasing it.
+if matches="$(
+  git grep -nE "$pattern" -- '*.rs' '*.nix' '*.js' '*.mjs' '*.ts' '*.tsx' '*.css' '*.html' '*.md' \
+    '*.yml' '*.yaml'
+)"; then
+  grep_status=0
+else
+  grep_status=$?
+fi
+
+if [ "$grep_status" -gt 1 ]; then
+  printf 'check-issue-refs: git grep failed while scanning for tracker references (exit %s)\n' "$grep_status" >&2
+  exit 1
+fi
+
+hits=""
+if [ "$grep_status" -eq 0 ]; then
+  # Lines carrying the `lint:allow` marker are dropped (legitimate
+  # non-tracker hit; see the header). Same exit-code split applies here:
+  # 1 means every match was exempted (clean), 2+ means grep itself broke.
+  if hits="$(printf '%s\n' "$matches" | grep -v 'lint:allow')"; then
+    filter_status=0
+  else
+    filter_status=$?
+  fi
+  if [ "$filter_status" -gt 1 ]; then
+    printf 'check-issue-refs: grep failed while filtering lint:allow markers (exit %s)\n' "$filter_status" >&2
+    exit 1
+  fi
+fi
 
 if [ -n "$hits" ]; then
   echo "$hits" | while IFS=: read -r file lineno _; do