From 5962604241ce7be8812c93cf5729c4121b0d3086 Mon Sep 17 00:00:00 2001 From: Austin Turner Date: Sat, 10 Oct 2026 11:18:38 -0500 Subject: [PATCH 1/2] fix(hooks): check the whole codebase as staged in pre-commit The hook passed staged paths to oxfmt and oxlint, which read the working tree. A file formatted on disk but never re-staged could slip through, and unstaged edits could block a clean commit. It now exports the index to a temporary directory and checks every tracked file there, so it sees exactly what the commit will contain. The header names all four repos sharing the hook, so every copy is byte-identical. --- .githooks/pre-commit | 49 +++++++++++++++++++++++--------------------- AGENTS.md | 8 +++++--- 2 files changed, 31 insertions(+), 26 deletions(-) diff --git a/.githooks/pre-commit b/.githooks/pre-commit index 5aa4654..a61d20f 100755 --- a/.githooks/pre-commit +++ b/.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/soql-parser-js - 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,44 @@ 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 ` too, and checkout-index honours it. Every +# tracked file is exported, so the whole codebase is checked, not only the files being changed. +snapshot=$(mktemp -d "${TMPDIR:-/tmp}/pre-commit.XXXXXX") || exit 1 +trap 'rm -rf "$snapshot"' EXIT +trap 'exit 1' HUP INT TERM -if [ -z "$staged" ]; then - exit 0 -fi +git -C "$root" checkout-index --all --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 diff --git a/AGENTS.md b/AGENTS.md index f0c2616..8834601 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -36,9 +36,11 @@ sequential polling loop in `scripts/`, and the data-driven Vitest suites in `src ## 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 From 2fbbb34a0adf3099655ca527a1b0f01d92bcfc61 Mon Sep 17 00:00:00 2001 From: Austin Turner Date: Sun, 11 Oct 2026 09:16:28 -0500 Subject: [PATCH 2/2] fix(hooks): export skip-worktree entries into the pre-commit snapshot Also normalize the snapshot path to an absolute one before installing the EXIT trap. A relative TMPDIR made mktemp return a relative path, which the trap then resolved from inside the snapshot after the cd and so never removed. The hook runs from the worktree root, so prefixing $PWD is exact. AGENTS.md now names simple-excel and simple-xml alongside soql-parser-js as the repositories the hook, release scripts and workflows are shared with, matching the hook header. --- .githooks/pre-commit | 9 +++++++-- AGENTS.md | 8 +++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/.githooks/pre-commit b/.githooks/pre-commit index a61d20f..e807b20 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -24,12 +24,17 @@ done # 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 ` too, and checkout-index honours it. Every -# tracked file is exported, so the whole codebase is checked, not only the files being changed. +# 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 and so never remove. The hook runs from the worktree root. +case $snapshot in /*) ;; *) snapshot=$PWD/$snapshot ;; esac trap 'rm -rf "$snapshot"' EXIT trap 'exit 1' HUP INT TERM -git -C "$root" checkout-index --all --prefix="$snapshot/" || exit 1 +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 diff --git a/AGENTS.md b/AGENTS.md index 8834601..8a24c88 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -111,6 +111,8 @@ Release commits pass `--no-verify`: they are machine generated from already chec hook failing mid-release would abort after the npm publish. `scripts/derive-increment.mjs`, `scripts/release.mjs`, `.githooks/pre-commit` and the -`.github/workflows/ci.yml`, `release.yml` and `changelog.yml` workflows are shared verbatim -with [soql-parser-js](https://github.com/jetstreamapp/soql-parser-js) — keep the copies in sync when -changing them. +`.github/workflows/ci.yml`, `release.yml` and `changelog.yml` workflows are shared verbatim with +[soql-parser-js](https://github.com/jetstreamapp/soql-parser-js), +[simple-excel](https://github.com/jetstreamapp/simple-excel) and +[simple-xml](https://github.com/jetstreamapp/simple-xml) — keep every copy in sync when changing +them.