From a5a3b823e38752eff3a4184c8149177481d0673d Mon Sep 17 00:00:00 2001 From: Angelica Willianto Date: Wed, 23 Sep 2026 18:03:45 +0800 Subject: [PATCH 1/2] chore(hooks): run push checks from one lefthook config --- .githooks/pre-push | 90 --------------- .github/workflows/hooks.yml | 57 ++++++++++ .github/workflows/markdown_lint.yml | 6 +- .github/workflows/rules_audit.yml | 55 ---------- README.md | 5 +- docs/rules/DEV-380.md | 21 ++-- lefthook.yml | 32 ++++++ package-lock.json | 164 ++++++++++++++++++++++++++++ package.json | 3 +- 9 files changed, 276 insertions(+), 157 deletions(-) delete mode 100755 .githooks/pre-push create mode 100644 .github/workflows/hooks.yml delete mode 100644 .github/workflows/rules_audit.yml create mode 100644 lefthook.yml diff --git a/.githooks/pre-push b/.githooks/pre-push deleted file mode 100755 index 5f06586..0000000 --- a/.githooks/pre-push +++ /dev/null @@ -1,90 +0,0 @@ -#!/usr/bin/env bash - -# Exit immediately on error, treat unset variables as errors, fail on pipe errors -set -euo pipefail - -ROOT=$(git rev-parse --show-toplevel) -RUMDL="$ROOT/node_modules/.bin/rumdl" - -# Intentionally only use the npm-pinned rumdl (package.json devDependency), -# never a globally installed binary (e.g. via cargo/brew) on PATH. Different -# rumdl versions format markdown differently, so falling back to whatever -# version a dev happens to have installed causes the pre-push hook to -# reformat files inconsistently across machines and block pushes with -# unrelated formatting diffs. See: -# https://github.com/holdex/marketing-website/issues/1138 -if [ ! -x "$RUMDL" ]; then - echo "rumdl not found — run npm install in the repo root" - exit 1 -fi - -cd "$ROOT" - -# git passes one line per ref being pushed via stdin: -# -while IFS=' ' read -r local_ref local_sha remote_ref remote_sha; do - # All-zeros local SHA means the ref is being deleted — nothing to check - if [ "$local_sha" = "0000000000000000000000000000000000000000" ]; then - continue - fi - - if [ "$remote_sha" = "0000000000000000000000000000000000000000" ]; then - # All-zeros remote SHA means the branch doesn't exist on the remote yet. - # Diffing against git's empty-tree object would treat every file in the repo - # as "added" and lint the whole tree, blocking the push on unrelated - # pre-existing formatting debt. Instead, scope to only what this branch - # changed relative to its base: the merge-base against the default branch. - # Fall back to the empty-tree object if that can't be resolved (e.g. no - # origin/HEAD and no origin/main), so a first push is never left unchecked. - default_ref=$(git symbolic-ref --quiet --short refs/remotes/origin/HEAD 2>/dev/null || echo "origin/main") - base=$(git merge-base "$default_ref" "$local_sha" 2>/dev/null || echo "4b825dc642cb6eb9a060e54bf8d69288fbee4904") - else - # Branch already exists — only check files changed in the commits being pushed - base="$remote_sha" - fi - - # List markdown files touched by the commits about to be pushed. - # --diff-filter=ACMR keeps Added/Copied/Modified/Renamed (the new path) and - # excludes Deleted, so renamed-away or removed files aren't linted (they no - # longer exist on disk and would fail rumdl with "File not found"). - files=$(git diff --name-only --diff-filter=ACMR "$base" "$local_sha" -- '*.md' 2>/dev/null || true) - - if [ -n "$files" ]; then - echo "$files" | xargs "$RUMDL" check --fix - - # If rumdl modified any of the linted files the working tree will be dirty — - # block the push. Scope the check to the linted files so unrelated uncommitted - # changes elsewhere in the tree are not misattributed to rumdl. - if ! git diff --quiet -- $files; then - echo "" - echo "rumdl check --fix modified the following files — commit the fixes and push again:" - git diff --name-only -- $files - exit 1 - fi - fi - - # When any doc changed, audit the rules system and the docs tree. The system - # checks are holistic (cross-file: dependency resolution, index completeness, - # and reachability from the root README), so a change to one file can orphan or - # break an invariant in another, and they always run over the whole tree. Query - # git directly (not the ACMR-filtered $files) so a deletion, e.g. removing a - # rule, still triggers the audit and cannot silently break a dependency or - # index link. - # - # The changed rule files are passed after --rules. The script checks the - # authoring standard of an individual rule only for the rules named there, so a - # push is blocked by the rules it touches, never by pre-existing ones it does - # not. Rules are brought up to standard as they are edited. The flag is always - # passed, even when the list is empty: without it the script runs a full audit, - # so a push that changes a doc but no rule would be blocked by every - # pre-existing rule in the tree. - docs_changed=$(git diff --name-only "$base" "$local_sha" -- 'docs/' 'README.md' 2>/dev/null || true) - if [ -n "$docs_changed" ]; then - changed_rules=$(git diff --name-only --diff-filter=ACMR "$base" "$local_sha" -- 'docs/rules/*.md' 2>/dev/null || true) - # Unquoted on purpose: the newline-separated list becomes one argument per - # rule file. Paths in this repo have no spaces. - node "$ROOT/scripts/check-rules.mjs" --rules $changed_rules - fi -done - -exit 0 diff --git a/.github/workflows/hooks.yml b/.github/workflows/hooks.yml new file mode 100644 index 0000000..78aed86 --- /dev/null +++ b/.github/workflows/hooks.yml @@ -0,0 +1,57 @@ +name: Run Git Hooks + +# Runs the pre-push hooks from lefthook.yml on the files this PR changes. +# A local hook can be skipped with --no-verify or never installed, so this is +# the check that cannot be bypassed. Same config as local, nothing to sync. + +on: + pull_request: + branches: + - main + paths: + - "**.md" + - "docs/**" + - "lefthook.yml" + - "scripts/check-rules.mjs" + - "rules.config.yml" + - "package.json" + - "package-lock.json" + types: + - opened + - synchronize + - reopened + - ready_for_review + +jobs: + hooks: + # Drafts are work in progress. The hooks start mattering when the PR is + # ready for review; the ready_for_review event runs them the moment it is. + if: github.event.pull_request.draft == false + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + # lefthook diffs against the merge base, so it needs the history. + fetch-depth: 0 + + - uses: actions/setup-node@v4 + with: + node-version: 20 + cache: npm + + - run: npm ci + + - name: Run the pre-push hooks + env: + BASE_REF: ${{ github.event.pull_request.base.ref }} + run: | + # checkout does not set origin/HEAD. Without it lefthook cannot find + # the base branch and lints unrelated files. + git remote set-head origin "$BASE_REF" + node_modules/.bin/lefthook run pre-push + + - name: Audit the rules system + # lefthook skips every command when a PR only deletes files, and a + # deleted rule is what breaks dependencies and the index. This runs the + # cross-file checks on every PR, with no file list to get wrong. + run: node scripts/check-rules.mjs --rules diff --git a/.github/workflows/markdown_lint.yml b/.github/workflows/markdown_lint.yml index 3f43cdf..73df513 100644 --- a/.github/workflows/markdown_lint.yml +++ b/.github/workflows/markdown_lint.yml @@ -1,4 +1,4 @@ -name: Validate Markdown and PR naming +name: Validate PR naming on: pull_request: @@ -30,6 +30,8 @@ jobs: - uses: ./.holdex-actions/.github/actions/composed/pr-checks with: run-prettier: false - run-markdown: true + # Markdown lint runs from lefthook.yml in hooks.yml, with the same + # pinned rumdl as the local pre-push hook. + run-markdown: false run-commits: true package-manager: bun diff --git a/.github/workflows/rules_audit.yml b/.github/workflows/rules_audit.yml deleted file mode 100644 index 7cc5827..0000000 --- a/.github/workflows/rules_audit.yml +++ /dev/null @@ -1,55 +0,0 @@ -name: Audit Rules - -# The pre-push hook runs the same audit, but a hook can be skipped with -# --no-verify and is not installed on every machine a branch is pushed from. -# This is the check that cannot be bypassed. - -on: - pull_request: - branches: - - main - paths: - - "docs/**" - - "README.md" - - "scripts/check-rules.mjs" - - "rules.config.yml" - types: - - opened - - synchronize - - reopened - - ready_for_review - -jobs: - audit: - # Drafts are work in progress; a rule is expected to be half written there. - # The audit starts mattering when the PR is ready for review, and the - # ready_for_review event above runs it the moment it is marked ready. - if: github.event.pull_request.draft == false - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - with: - # Both sides of the comparison are needed to list the changed rules. - fetch-depth: 0 - - - uses: actions/setup-node@v4 - with: - node-version: 20 - - - name: Audit the rules system - env: - BASE_REF: ${{ github.event.pull_request.base.ref }} - run: | - # Compare against the merge base, not the base branch tip. A plain - # two-dot diff against the tip reports every file the base branch - # moved on since this PR forked, which would pull rules the PR never - # touched into scope and fail it on somebody else's work. - base=$(git merge-base "origin/$BASE_REF" HEAD) - - # Mirror the pre-push hook: system invariants across the whole tree, - # the authoring standard only on the rules this PR touches. --rules is - # always passed, empty list included, because without it the script - # runs a full audit and a docs-only PR would fail on every rule that - # predates the audit. - changed_rules=$(git diff --name-only --diff-filter=ACMR "$base" HEAD -- 'docs/rules/*.md') - node scripts/check-rules.mjs --rules $changed_rules diff --git a/README.md b/README.md index db23de6..f306940 100644 --- a/README.md +++ b/README.md @@ -12,8 +12,9 @@ improvements. ### Local After cloning, run `npm install` once to install the pinned `rumdl` version and -enable the markdown lint and rules-audit checks on push (`postinstall` sets -`core.hooksPath` to `.githooks` automatically). +enable the markdown lint and rules-audit checks on push (`postinstall` runs +`lefthook install`). The hooks live in `lefthook.yml`, and CI runs the same +file. ### Stage / Preview diff --git a/docs/rules/DEV-380.md b/docs/rules/DEV-380.md index 880c69d..7e6d645 100644 --- a/docs/rules/DEV-380.md +++ b/docs/rules/DEV-380.md @@ -18,15 +18,22 @@ Make lint automatic and version-stable, from the npm-pinned `rumdl` only. 1. Pin `rumdl` as a devDependency, so everyone runs the same version and avoids the version drift that blocks pushes with unrelated reformatting. -1. In `postinstall`, set `git config core.hooksPath .githooks`, so the hook - installs on `npm install`. -1. Add a `.githooks/pre-push` that runs the pinned binary - (`node_modules/.bin/rumdl`), never a global one, over the markdown changed in - the push, and blocks the push if it reformats anything. +1. Install the hooks with [lefthook](https://lefthook.dev), pinned as a + devDependency, and run `lefthook install` in `postinstall`, so the hooks + install on `npm install`. A hand-written hook script re-implements which + files a push changes, and that logic drifts and breaks on new branches. +1. In `lefthook.yml`, add a `pre-push` command that runs the pinned binary + (`node_modules/.bin/rumdl check --fix`), never a global one, over + `{push_files}`, and set `fail_on_changes: always` so the push is blocked if + it reformats anything. +1. Run the same `lefthook.yml` in CI, so a push that skipped the hook is still + checked, and the check lives in one place. ### Acceptance Criteria - [ ] `rumdl` is pinned in `devDependencies` -- [ ] `postinstall` sets `core.hooksPath` to `.githooks` -- [ ] `.githooks/pre-push` runs `node_modules/.bin/rumdl`, not a global binary +- [ ] `postinstall` runs `lefthook install` +- [ ] `lefthook.yml` runs `node_modules/.bin/rumdl` on `{push_files}`, not a + global binary +- [ ] CI runs the same `lefthook.yml` - [ ] A push containing a lint violation is blocked until it is fixed diff --git a/lefthook.yml b/lefthook.yml new file mode 100644 index 0000000..808c388 --- /dev/null +++ b/lefthook.yml @@ -0,0 +1,32 @@ +# Git hooks for this repo, managed by lefthook (https://lefthook.dev). +# `npm install` installs them. This file is the one place to edit hook logic: +# git runs it on every push, and CI runs the same commands on every PR. + +pre-push: + # rumdl --fix rewrites files in place. Fail the push when it did, so the + # fixes get committed instead of pushed around. + fail_on_changes: always + fail_on_changes_diff: true + commands: + markdown: + # {push_files} is what this push adds. On a branch with no upstream yet + # lefthook diffs against the default branch, not the whole repo. + glob: "*.md" + # The npm-pinned rumdl only, never a global one: versions format + # differently and would block pushes with unrelated reformatting. + run: node_modules/.bin/rumdl check --fix {push_files} + priority: 1 + rules: + # Authoring standard for the rules this push touches, so a push is + # blocked by its own rules, never by pre-existing ones. + glob: "docs/rules/*.md" + run: node scripts/check-rules.mjs --rules {push_files} + priority: 2 + rules-system: + # Cross-file invariants (dependencies, index, reachability) over the whole + # tree, on any push. `--rules` with no names keeps the authoring standard + # out of scope. lefthook skips every pre-push command when a push only + # deletes files, so hooks.yml also runs this outside lefthook: deleting a + # rule is exactly what breaks these invariants. + run: node scripts/check-rules.mjs --rules + priority: 3 diff --git a/package-lock.json b/package-lock.json index 07b704f..d13d27d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,6 +9,7 @@ "version": "0.0.1", "hasInstallScript": true, "devDependencies": { + "lefthook": "2.1.14", "rumdl": "0.2.27" } }, @@ -131,6 +132,169 @@ "node": ">=18.0.0" } }, + "node_modules/lefthook": { + "version": "2.1.14", + "resolved": "https://registry.npmjs.org/lefthook/-/lefthook-2.1.14.tgz", + "integrity": "sha512-Y9TL1dmpRlIdSp73SJ6bb+xAZ4s2SinGTK3pgC9PRWMSjMr2iOfowPGnlSOfAo5fu6S5rdiItsQDxeLEWCGk4g==", + "dev": true, + "hasInstallScript": true, + "license": "MIT", + "bin": { + "lefthook": "bin/index.js" + }, + "optionalDependencies": { + "lefthook-darwin-arm64": "2.1.14", + "lefthook-darwin-x64": "2.1.14", + "lefthook-freebsd-arm64": "2.1.14", + "lefthook-freebsd-x64": "2.1.14", + "lefthook-linux-arm64": "2.1.14", + "lefthook-linux-x64": "2.1.14", + "lefthook-openbsd-arm64": "2.1.14", + "lefthook-openbsd-x64": "2.1.14", + "lefthook-windows-arm64": "2.1.14", + "lefthook-windows-x64": "2.1.14" + } + }, + "node_modules/lefthook-darwin-arm64": { + "version": "2.1.14", + "resolved": "https://registry.npmjs.org/lefthook-darwin-arm64/-/lefthook-darwin-arm64-2.1.14.tgz", + "integrity": "sha512-VMUAqY81+8ph7Mc0iROd3JWqj2sOuK7SYDyd8ZgzYVKChCt+Nnz8ehd4gUwprJy7QI28IFSUh1M3wnIRiYh+fA==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ] + }, + "node_modules/lefthook-darwin-x64": { + "version": "2.1.14", + "resolved": "https://registry.npmjs.org/lefthook-darwin-x64/-/lefthook-darwin-x64-2.1.14.tgz", + "integrity": "sha512-uSmjcPRU+yB+D6vk+u6WJDnyHlB+XIv1QZw97N2+qNGG2bNSLH/7MuJ6puCdJv+lVPBEO5wBjE3JIbaEzPOQCw==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ] + }, + "node_modules/lefthook-freebsd-arm64": { + "version": "2.1.14", + "resolved": "https://registry.npmjs.org/lefthook-freebsd-arm64/-/lefthook-freebsd-arm64-2.1.14.tgz", + "integrity": "sha512-po/pmjbp/7BjE9RFfeDnh+CeNsA0d+utppOYKB425I7mgI1GcmT8GAZ9DDut4DIutNjVqKL/lzrj+F87Gm40Rg==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "freebsd" + ] + }, + "node_modules/lefthook-freebsd-x64": { + "version": "2.1.14", + "resolved": "https://registry.npmjs.org/lefthook-freebsd-x64/-/lefthook-freebsd-x64-2.1.14.tgz", + "integrity": "sha512-+KiXiFjJf7YdHUYjzhDsklcA5bAFZN+pNcz/NbBftrhVTwzNALNoK+SYEKt+9QJsrzL6tpDWmHHFpeLGp+8t8w==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "freebsd" + ] + }, + "node_modules/lefthook-linux-arm64": { + "version": "2.1.14", + "resolved": "https://registry.npmjs.org/lefthook-linux-arm64/-/lefthook-linux-arm64-2.1.14.tgz", + "integrity": "sha512-bjeJB16ftJaPWGTedssh8ZrqRy1ACfQuTV7a0O3NU1FoIsJtfHOm0o0mV4cPf4q+ZCVCwq1Xf2I11qUpuJFV4Q==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "linux" + ] + }, + "node_modules/lefthook-linux-x64": { + "version": "2.1.14", + "resolved": "https://registry.npmjs.org/lefthook-linux-x64/-/lefthook-linux-x64-2.1.14.tgz", + "integrity": "sha512-gYExzjU2w3uKrP1MklbHENS1cXUdCQRIiEJkykI7q4RjhIHi+mQEUAZLxZw93a6jE2pNWaO0me5tB8EbEt4/GQ==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "linux" + ] + }, + "node_modules/lefthook-openbsd-arm64": { + "version": "2.1.14", + "resolved": "https://registry.npmjs.org/lefthook-openbsd-arm64/-/lefthook-openbsd-arm64-2.1.14.tgz", + "integrity": "sha512-MCW76gTlEMBF3Ds8O63UTxjaSguha/5zT0U3Vr719I1sXueSpttfRmm/5YfIB4MygB116yA/Vi+DsizT/X0kcg==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "openbsd" + ] + }, + "node_modules/lefthook-openbsd-x64": { + "version": "2.1.14", + "resolved": "https://registry.npmjs.org/lefthook-openbsd-x64/-/lefthook-openbsd-x64-2.1.14.tgz", + "integrity": "sha512-salAc1PAhLI6xrB9pth5UxALQlTOGvZAfaAvgFwixhkdu8D//uW8zS3faFq31ktd1bDAy5wDI8p9yg+fF5+2uA==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "openbsd" + ] + }, + "node_modules/lefthook-windows-arm64": { + "version": "2.1.14", + "resolved": "https://registry.npmjs.org/lefthook-windows-arm64/-/lefthook-windows-arm64-2.1.14.tgz", + "integrity": "sha512-q3JC6lBrYLzkhXEhfnmFxJF47gSd5JeDweH3aNrHoLGX/KkRiOrk9RSyoRwSuGq03EYxSkJhl1FLj2GUxF7VqQ==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "win32" + ] + }, + "node_modules/lefthook-windows-x64": { + "version": "2.1.14", + "resolved": "https://registry.npmjs.org/lefthook-windows-x64/-/lefthook-windows-x64-2.1.14.tgz", + "integrity": "sha512-WwIxdwW1lDAaJUU3ETcKlCtdqluGr3Cy+LjKvP4QIQnFf0FEXL78THstIkZ6k6qhfUn7/WvyqSIf0cbTAKw2nQ==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "win32" + ] + }, "node_modules/rumdl": { "version": "0.2.27", "resolved": "https://registry.npmjs.org/rumdl/-/rumdl-0.2.27.tgz", diff --git a/package.json b/package.json index f867e0b..aefb0e0 100644 --- a/package.json +++ b/package.json @@ -3,10 +3,11 @@ "version": "0.0.1", "private": true, "scripts": { - "postinstall": "git config core.hooksPath .githooks", + "postinstall": "git config --unset-all core.hooksPath || true; lefthook install", "check:rules": "node scripts/check-rules.mjs" }, "devDependencies": { + "lefthook": "2.1.14", "rumdl": "0.2.27" } } From 57ddb44f979180862e547afca199944247220efa Mon Sep 17 00:00:00 2001 From: Angelica Willianto Date: Wed, 23 Sep 2026 18:17:18 +0800 Subject: [PATCH 2/2] Update hooks.yml --- .github/workflows/hooks.yml | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/hooks.yml b/.github/workflows/hooks.yml index 78aed86..aac68d7 100644 --- a/.github/workflows/hooks.yml +++ b/.github/workflows/hooks.yml @@ -31,7 +31,7 @@ jobs: steps: - uses: actions/checkout@v4 with: - # lefthook diffs against the merge base, so it needs the history. + # lefthook diffs against the base branch, so it needs its commits. fetch-depth: 0 - uses: actions/setup-node@v4 @@ -45,9 +45,12 @@ jobs: env: BASE_REF: ${{ github.event.pull_request.base.ref }} run: | - # checkout does not set origin/HEAD. Without it lefthook cannot find - # the base branch and lints unrelated files. + # lefthook reads the base branch from origin/HEAD, which checkout + # does not set, then diffs against the LOCAL branch of that name, + # which checkout does not create. The PR checkout is the merge of + # this PR into the base, so that diff is exactly this PR's changes. git remote set-head origin "$BASE_REF" + git branch "$BASE_REF" "origin/$BASE_REF" node_modules/.bin/lefthook run pre-push - name: Audit the rules system