fix(hooks): hooks that never run, and the guards that cannot see them (#14909, #14891, #14884)

.github/filters/python-paths.yml

@@ -45,3 +45,26 @@ python:
   - 'autobot-slm-backend/ansible/roles/slm_manager/templates/autobot-slm-backend.service.j2'
   - 'autobot-slm-backend/ansible/roles/slm_agent/templates/slm-agent.service.j2'
   - 'autobot-slm-backend/ansible/roles/ai-stack/templates/ai-stack.env.j2'
+  # #14909/#14891: the same shape again, for the hook guards. Three repo_tests
+  # assert on inputs that contain no Python at all — git hook INDEX MODES
+  # (git_hooks_executable_test.py), the copy-based installer
+  # (git_hooks_installer_test.py) and every shell script under
+  # autobot-infrastructure/ (shell_lib_sources_resolve_test.py). Without these
+  # entries, flipping tools/git-hooks/pre-push back to 100644, or reintroducing
+  # `|| true` in an extensionless hook, touches no `**/*.py` path: the shim
+  # computes `python != 'true'`, reports `python-suite` green, and the guard
+  # written to catch exactly that never runs. A mode-only change still appears
+  # in the changed-file list, so these entries do gate it.
+  # `autobot-infrastructure/**`, not a narrower shell-shaped pattern, on
+  # purpose. shell_lib_sources_resolve_test.py finds its subjects by SHEBANG,
+  # and "has a shebang naming a shell" cannot be written as a glob: a new
+  # extensionless script added anywhere under this tree would match neither
+  # `**/*.sh` nor `shared/scripts/hooks/**`, take the shim's green, and sit
+  # outside the guard exactly as the 16 hooks did before #14891. The narrow
+  # patterns happened to cover 100% of the surface on the day they were
+  # written, which is the property that decays silently. The cost is that an
+  # infrastructure-only change now runs python-suite; the alternative is a
+  # filter that is narrower than the guard it gates.
+  - 'autobot-infrastructure/**'
+  - 'tools/git-hooks/**'
+  - 'scripts/install-git-hooks.sh'

.github/workflows/ci.yml

@@ -278,7 +278,7 @@ jobs:
         # quarantine. scripts/lib/*_test.sh are bash and are covered separately
         # by repo_tests/shell_lib_test.py.
         python -m pytest \
-          autobot-backend autobot_shared autobot-tts-worker repo_tests tools scripts pipeline-scripts \
+          autobot-backend autobot_shared autobot-tts-worker repo_tests tools scripts pipeline-scripts autobot-infrastructure/shared/scripts/hooks \
           -n auto --dist loadscope \
           -p repo_tests.stable_shard \
           --shard-splits 12 --shard-group ${{ matrix.shard }} \

.github/workflows/coverage.yml

@@ -74,7 +74,7 @@ jobs:
       - name: Run unit tests with coverage — backend, shared, tts-worker, repo_tests
         run: |
           python -m pytest \
-            autobot-backend autobot_shared autobot-tts-worker repo_tests \
+            autobot-backend autobot_shared autobot-tts-worker repo_tests autobot-infrastructure/shared/scripts/hooks \
             -n auto --dist loadscope \
             --splits 12 --group ${{ matrix.shard }} \
             --durations-path .test_durations \

.github/workflows/test-durations.yml

@@ -80,7 +80,7 @@ jobs:
       - name: Store durations — backend, shared, tts-worker, repo_tests
         run: |
           python -m pytest \
-            autobot-backend autobot_shared autobot-tts-worker repo_tests \
+            autobot-backend autobot_shared autobot-tts-worker repo_tests autobot-infrastructure/shared/scripts/hooks \
             -n auto --dist loadscope \
             -m "not integration and not slow and not distributed and not performance" \
             --store-durations \

autobot-infrastructure/shared/scripts/hooks/post-commit-doc-sync

@@ -13,7 +13,15 @@ set -e
 
 SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 # shellcheck source=lib/_common.sh
-source "$SCRIPT_DIR/lib/_common.sh"
+# GH#14891: this used to be a bare `source` with no failure block at all.
+# It happened to fail closed only because of the `set -e` above, which is
+# incidental — a source evaluated inside a condition or a function would
+# not have aborted, and every ${CYAN}/${NC} below would then be empty under
+# `set -u`. The failure is now handled where it happens.
+source "$SCRIPT_DIR/lib/_common.sh" || {
+    echo "post-commit-doc-sync: cannot load lib/_common.sh — refusing to report clean" >&2
+    exit 1
+}
 
 # Get the project root directory
 PROJECT_ROOT="$(git rev-parse --show-toplevel)"

autobot-infrastructure/shared/scripts/hooks/pre-commit-no-tag-pinned-action_test.py

@@ -23,12 +23,27 @@
 
 
 def _run(tmp_path: Path, content: str, rel: str = ".github/workflows/test.yml") -> subprocess.CompletedProcess:
-    """Write a workflow file and invoke the hook via argv mode."""
+    r"""Write a workflow file and invoke the hook via argv mode.
+
+    The path goes in REPO-RELATIVE with cwd at its root, because that is the
+    only shape a real caller produces: pre-commit and the CI wrappers both hand
+    the hook paths relative to the repository root.
+
+    GH#14884: this used to pass ``str(f)`` — an absolute ``/tmp/pytest-.../...``
+    path. GH#13936 made ``get_staged_files`` apply the caller's own pattern to
+    the argv branch as well as the staged branch, and this hook's pattern is
+    anchored: ``^\.github/(workflows|actions)/.*\.ya?ml$``. An absolute path
+    cannot match that anchor, so the hook received an empty file list, took its
+    ``[ -z "$files" ] && exit 0`` path and reported clean. The seven "must
+    reject" tests read that as a missing rejection and went red; worse, the
+    three "must allow" tests went green having exercised nothing at all.
+    """
     f = tmp_path / rel
     f.parent.mkdir(parents=True, exist_ok=True)
     f.write_text(content, encoding="utf-8")
     return subprocess.run(
-        [str(HOOK_PATH), str(f)],
+        [str(HOOK_PATH), rel],
+        cwd=tmp_path,
         capture_output=True,
         text=True,
     )

autobot-infrastructure/shared/scripts/hooks/pre-commit-warn-untracked

@@ -46,10 +46,16 @@ SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 # dependency and "nothing to warn about" would be byte-identical. So it says
 # so on stderr first. That keeps "never blocks a commit" (still exit 0) while
 # refusing to report clean about a check that never ran.
-if ! source "$SCRIPT_DIR/lib/_common.sh" 2>/dev/null; then
+# GH#14891: the decisive attempt used to be written `if ! source ... 2>/dev/null`,
+# which threw away the library's OWN stderr — the developer was told the load
+# failed but never why. The redirect is gone; the exit 0 stays, because it is
+# this hook's contract rather than an oversight, and the marker below is what
+# the guard reads instead of an allowlist of filenames.
+source "$SCRIPT_DIR/lib/_common.sh" || {
+    # lib-source-contract: non-blocking
     echo "warn-untracked: cannot load lib/_common.sh — untracked-file check SKIPPED (not clean)" >&2
     exit 0
-fi
+}
 
 SOURCE_PATTERN='\.py$|\.ts$|\.vue$|\.js$'
 

autobot-infrastructure/shared/scripts/hooks/pre-commit-worktree-branch-guard_test.py

@@ -20,6 +20,7 @@
 from __future__ import annotations
 
 import os
+import shutil
 import stat
 import subprocess
 from pathlib import Path
@@ -78,13 +79,14 @@ class TestFailsClosedWhenGitWorktreeListFails:
     """
 
     def _make_fake_git(self, tmp_path: Path) -> Path:
-        real_git = subprocess.run(["command", "-v", "git"], shell=False, capture_output=True, text=True)
-        # Resolve the real git path without relying on a shell builtin.
-        real_git_path = None
-        for candidate in ("/usr/bin/git", "/bin/git", "/usr/local/bin/git"):
-            if Path(candidate).exists():
-                real_git_path = candidate
-                break
+        # GH#14884: this used to resolve git with
+        # `subprocess.run(["command", "-v", "git"])`. `command` is a POSIX shell
+        # BUILTIN, not a binary, so subprocess raised FileNotFoundError on every
+        # machine with no `command` executable on PATH — before the hardcoded
+        # candidate list below it ever ran, and its result was never read anyway.
+        # That left this fail-closed property unverified. shutil.which answers
+        # the same question in-process, and honours PATH instead of guessing.
+        real_git_path = shutil.which("git")
         assert real_git_path, "could not locate a real git binary to delegate to"
 
         fake_bin = tmp_path.parent / "fake-bin"

autobot-infrastructure/shared/scripts/hooks/slm-post-commit

@@ -1,7 +1,7 @@
+#!/bin/bash
 # AutoBot - AI-Powered Automation Platform
 # Copyright (c) 2025 mrveiss
 # Author: mrveiss
-#!/bin/bash
 #
 # Git post-commit hook for SLM code version tracking (Issue #741)
 #

pytest.ini

@@ -64,6 +64,22 @@ testpaths =
     # test that had been red for as long as the bug it covered went unnoticed.
     # A guard's own test must run, or the guard is unverified twice over.
     pipeline-scripts
+    # #14884: the same defect a third time, and the sharpest case of it. The 18
+    # suites under autobot-infrastructure/shared/scripts/hooks/ are the ONLY
+    # tests of the pre-commit hooks themselves, and nothing collected them: not
+    # this list, not any of the three CI invocations, and not
+    # autobot-infrastructure/shared/tests/pytest.ini (whose `testpaths = unit
+    # integration e2e` does not reach shared/scripts/). So `git worktree list`
+    # failing closed, and the supply-chain check on tag-pinned third-party
+    # actions, were both guarded by tests that ran nowhere. 159 tests, measured
+    # green before wiring — this is a wiring fix, not a quarantine.
+    #
+    # NARROW ON PURPOSE. `autobot-infrastructure` as a whole does NOT collect:
+    # 23 collection errors, and shared/scripts/test_alertmanager.py calls
+    # sys.exit(0) at import time, which takes pytest itself down with an
+    # INTERNALERROR. That is a separate piece of work, filed rather than
+    # bundled here.
+    autobot-infrastructure/shared/scripts/hooks
 # #7082: dropped `*.e2e_test.py` / `*.integration_test.py` (dotted) — they
 # break pytest's path-to-module conversion. Files renamed to `*_e2e_test.py`
 # / `*_integration_test.py` / `*_performance_test.py` (underscore form),