Merge pull request #303 from jetstreamapp/fix/pre-commit-index-snapshot

fix(hooks): check the whole codebase as staged in pre-commit

Author: paustint

.githooks/pre-commit

@@ -1,15 +1,17 @@
 #!/bin/sh
-# Blocks a commit when any staged file is unformatted or fails lint.
+# Blocks a commit unless the whole codebase, exactly as it will be committed, is formatted and
+# passes lint.
 #
 # oxfmt is the formatter (.oxfmtrc.json); oxlint is the linter (.oxlintrc.json). Wired up by
 # the "prepare" script in package.json (git config core.hooksPath .githooks) on npm install.
 # Emergency bypass: git commit --no-verify
 #
-# This file is shared verbatim with jetstreamapp/sf-formula-parser - keep the two copies in
-# sync when changing it.
+# This file is shared verbatim by jetstreamapp/sf-formula-parser, jetstreamapp/soql-parser-js,
+# jetstreamapp/simple-excel and jetstreamapp/simple-xml - keep every copy in sync when changing it.
 
-OXFMT="./node_modules/.bin/oxfmt"
-OXLINT="./node_modules/.bin/oxlint"
+root=$(git rev-parse --show-toplevel) || exit 1
+OXFMT="$root/node_modules/.bin/oxfmt"
+OXLINT="$root/node_modules/.bin/oxlint"
 
 for bin in "$OXFMT" "$OXLINT"; do
   if [ ! -x "$bin" ]; then
@@ -18,43 +20,49 @@ for bin in "$OXFMT" "$OXLINT"; do
   fi
 done
 
-# Added, copied, modified and renamed paths only - deleted paths are not there to check.
-staged=$(git diff --cached --name-only --diff-filter=ACMR)
+# The checks run against a snapshot of the index rather than the working tree. The index is
+# exactly what the commit will contain, so a file formatted on disk but never re-staged cannot
+# slip through, and unstaged edits cannot block a clean commit. git points GIT_INDEX_FILE at the
+# right index for `commit -a` and `commit <path>` too, and checkout-index honours it. Every
+# tracked file is exported, so the whole codebase is checked, not only the files being changed;
+# --ignore-skip-worktree-bits includes paths a sparse checkout or `update-index --skip-worktree`
+# leaves out of the working tree.
+snapshot=$(mktemp -d "${TMPDIR:-/tmp}/pre-commit.XXXXXX") || exit 1
+# A relative TMPDIR yields a relative path, which the EXIT trap would resolve from inside the
+# snapshot after the cd below, leaving it behind - anchor it to where git ran the hook.
+case $snapshot in /*) ;; *) snapshot="$PWD/$snapshot" ;; esac
+trap 'rm -rf "$snapshot"' EXIT
+trap 'exit 1' HUP INT TERM
 
-if [ -z "$staged" ]; then
-  exit 0
-fi
+git -C "$root" checkout-index --all --ignore-skip-worktree-bits --prefix="$snapshot/" || exit 1
+
+# Some lint rules resolve imports, so the snapshot shares the installed dependencies.
+ln -s "$root/node_modules" "$snapshot/node_modules" || exit 1
 
-# Both tools skip what they do not handle, so the whole staged list goes to each rather than
-# being filtered by extension here, which would drift as the tools gain file types.
-# --no-error-on-unmatched-pattern: a staged list with nothing for that tool is not an error.
-# The list is newline delimited, so convert to NUL to keep paths containing spaces intact.
-check() {
-  printf '%s\n' "$staged" | tr '\n' '\0' | xargs -0 "$@" --no-error-on-unmatched-pattern
-}
+cd "$snapshot" || exit 1
 
 # Both run even when the first fails, so one commit attempt reports everything to fix.
 status=0
 
-if ! check "$OXFMT" --check; then
+if ! "$OXFMT" --check; then
   cat >&2 <<'MSG'
 
-Commit blocked: the staged files listed above are not formatted.
+Commit blocked: the files listed above are not formatted.
 
   npm run format     # format the repo
-  git add -u         # re-stage the result
+  git add -u         # stage the result
 MSG
   status=1
 fi
 
-if ! check "$OXLINT"; then
+if ! "$OXLINT"; then
   cat >&2 <<'MSG'
 
-Commit blocked: the staged files listed above have lint errors.
+Commit blocked: the files listed above have lint errors.
 
   npm run lint       # review the full report
   npm run lint:fix   # apply the fixes oxlint can make automatically
-  git add -u         # re-stage the result
+  git add -u         # stage the result
 MSG
   status=1
 fi

AGENTS.md

@@ -38,9 +38,11 @@ entry in `overrides` (by file glob) rather than disabling it repo-wide, and reco
 
 ## Commit hook
 
-A `pre-commit` hook in `.githooks/` runs both the format check and oxlint against staged files and
-**blocks the commit** if either fails, so skipping those steps will stop the commit rather than slip
-through. The hook is wired up by the `prepare` script (`git config core.hooksPath .githooks`) on
+A `pre-commit` hook in `.githooks/` runs both the format check and oxlint against the whole codebase
+and **blocks the commit** if either fails, so skipping those steps will stop the commit rather than
+slip through. It checks a snapshot of the index, not the working tree, so it sees exactly what the
+commit will contain: a file formatted on disk but not re-staged still blocks, and unstaged edits
+never do. The hook is wired up by the `prepare` script (`git config core.hooksPath .githooks`) on
 `npm install`. Emergency bypass: `git commit --no-verify`.
 
 ## Types