Skip to content

chore: add shared hook config and improve Husky scripts#662

Merged
cv merged 7 commits intoNVIDIA:mainfrom
HagegeR:pre-commit-checks
Mar 22, 2026
Merged

chore: add shared hook config and improve Husky scripts#662
cv merged 7 commits intoNVIDIA:mainfrom
HagegeR:pre-commit-checks

Conversation

@HagegeR
Copy link
Contributor

@HagegeR HagegeR commented Mar 22, 2026

Summary

Adds a repo-wide .pre-commit-config.yaml (shell, Docker, ruff, nemoclaw ESLint/Prettier, SPDX script, gitleaks, hygiene hooks) and documents prek as the recommended runner with pre-commit as an alternative.
Hardens Husky (shared scripts/husky-env.sh, SPDX on hook scripts, shellcheck-safe @{u} quoting) and fixes SC2206 in scripts/install-openshell.sh.
Goal: consistent checks locally and a clear path for CI later.

Changes

  • Add .pre-commit-config.yaml with priority groups (prek), excludes, and local nemoclaw TS hooks.
  • Update CONTRIBUTING.md: optional prek / pre-commit, table row, pre-push behavior.
  • Add scripts/husky-env.sh and source it from .husky/pre-commit, .husky/pre-push, .husky/commit-msg (PATH + nvm/fnm for GUI Git).
  • .husky/*: SPDX headers, SC1091 / @{u} fixes for shellcheck.
  • scripts/install-openshell.sh: shellcheck-safe version compare (read -a), mode/exec as needed.

Type of Change

  • Code change for a new feature, bug fix, or refactor.
  • Code change with doc updates.
  • Doc only. Prose changes without code sample modifications.
  • Doc only. Includes code sample changes.

Testing

  • make check passes.
  • npm test passes.
  • [] make docs builds without warnings. (for doc-only changes)

Checklist

General

Code Changes

  • make format applied (TypeScript and Python).
  • Tests added or updated for new or changed behavior.
  • No secrets, API keys, or credentials committed.
  • Doc pages updated for any user-facing behavior changes (new commands, changed defaults, new features, bug fixes that contradict existing docs).

Doc Changes

  • Follows the style guide. Try running the update-docs agent skill to draft changes while complying with the style guide. For example, prompt your agent with "/update-docs catch up the docs for the new changes I made in this PR."
  • New pages include SPDX license header and frontmatter, if creating a new page.
  • Cross-references and links verified.

How to verify

  • pip install pre-commit && pre-commit run --all-files
    (or: prek install && prek run --all-files)
  • git push (exercises pre-push if prek/pre-commit on PATH)

Summary by CodeRabbit

  • Chores
    • Enhanced Git hooks and development tooling to streamline code quality checks and improve the developer workflow.
    • Updated development configuration and documentation to simplify local environment setup and contribution processes.

@coderabbitai
Copy link

coderabbitai bot commented Mar 22, 2026

📝 Walkthrough

Walkthrough

These changes systematize the Git hook infrastructure by introducing a shared environment setup script, implementing conditional fallback logic for local and global tool invocation across all Husky hooks, and integrating a comprehensive pre-commit configuration with multiple validation and linting stages.

Changes

Cohort / File(s) Summary
Husky Git Hooks
.husky/commit-msg, .husky/pre-commit, .husky/pre-push
Added SPDX headers, repository root resolution, environment sourcing via scripts/husky-env.sh, and conditional local binary execution with npx fallback. Pre-push additionally includes type-checking logic and integration with prek/pre-commit for shared lint/format checks via git merge-base range computation.
Pre-commit Framework
.pre-commit-config.yaml
New configuration file defining repository-wide hooks across 5 priority levels: whitespace/EOF fixers (0), formatters including shfmt/ruff/Prettier (5), ESLint auto-fixes (6), and validators including shellcheck/hadolint/SPDX verification/gitleaks (10). Excludes generated, build, and dependency directories.
Documentation
CONTRIBUTING.md
Added guidance for optional prek/pre-commit setup, scoped linting execution via --from-ref/--to-ref, and clarification that .husky/pre-push runs shared hooks when tools are available.
Utility Scripts
scripts/husky-env.sh, scripts/install-openshell.sh
New husky-env.sh configures PATH and Node version managers (NVM, FNM) for git hook execution. Refactored install-openshell.sh bash functions with improved version string parsing via explicit array assignment and normalized command tests.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰 The hooks align, the scripts take flight,
With fallbacks ready, binaries tight,
Pre-commit guards the path ahead,
While husky whispers what must be said,
A toolchain robust, by design quite right! 🛠️✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'chore: add shared hook config and improve Husky scripts' accurately describes the main changes: adding .pre-commit-config.yaml and enhancing Husky hooks with improved scripts and environment handling.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

CodeRabbit can approve the review once all CodeRabbit's comments are resolved.

Enable the reviews.request_changes_workflow setting to automatically approve the review once all CodeRabbit's comments are resolved.

@ericksoa ericksoa requested a review from cv March 22, 2026 21:19
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (3)
.husky/pre-push (1)

31-55: Consider extracting the shared hook logic to reduce duplication.

The prek and pre-commit branches have identical logic for computing --from-ref and running the hooks. This could be extracted into a helper function, though the current explicit form is also readable.

♻️ Optional refactor to reduce duplication
+run_shared_hooks() {
+  tool="$1"
+  if git rev-parse '@{u}' >/dev/null 2>&1; then
+    FROM=$(git merge-base HEAD '@{u}')
+    "$tool" run --from-ref "$FROM" --to-ref HEAD
+  elif git rev-parse HEAD~1 >/dev/null 2>&1; then
+    "$tool" run --from-ref HEAD~1 --to-ref HEAD
+  else
+    "$tool" run --all-files
+  fi
+}
+
 # prek: same hooks as .pre-commit-config.yaml — optional if not installed locally.
 # Falls back to pre-commit if prek is absent (same config file).
 if command -v prek >/dev/null 2>&1; then
   echo "pre-push: running prek on outgoing commits..."
-  if git rev-parse '@{u}' >/dev/null 2>&1; then
-    FROM=$(git merge-base HEAD '@{u}')
-    prek run --from-ref "$FROM" --to-ref HEAD
-  elif git rev-parse HEAD~1 >/dev/null 2>&1; then
-    prek run --from-ref HEAD~1 --to-ref HEAD
-  else
-    prek run --all-files
-  fi
+  run_shared_hooks prek
 elif command -v pre-commit >/dev/null 2>&1; then
   echo "pre-push: running pre-commit on outgoing commits..."
-  if git rev-parse '@{u}' >/dev/null 2>&1; then
-    FROM=$(git merge-base HEAD '@{u}')
-    pre-commit run --from-ref "$FROM" --to-ref HEAD
-  elif git rev-parse HEAD~1 >/dev/null 2>&1; then
-    pre-commit run --from-ref HEAD~1 --to-ref HEAD
-  else
-    pre-commit run --all-files
-  fi
+  run_shared_hooks pre-commit
 else
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.husky/pre-push around lines 31 - 55, Extract the duplicated logic that
computes FROM and runs hooks into a reusable shell function (e.g.,
run_outgoing_commits or compute_from_and_run) and call it for both prek and
pre-commit branches; the function should accept the command name ("prek run" or
"pre-commit run") or the full command to execute, compute FROM using git
merge-base HEAD '@{u}' fallback to HEAD~1 or --all-files, and then invoke the
provided command with --from-ref "$FROM" --to-ref HEAD (or --all-files) so you
remove the repeated blocks that currently call prek run and pre-commit run and
reuse the single helper instead.
scripts/husky-env.sh (1)

1-2: Add a shell directive for shellcheck and documentation clarity.

Since this file is sourced (not executed directly), a shebang isn't strictly required. However, adding a shell directive comment helps shellcheck analyze correctly and documents the expected shell.

📝 Proposed fix
+# shellcheck shell=sh
 # SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/husky-env.sh` around lines 1 - 2, Add a shell directive comment to
help shellcheck and document the expected shell for this sourced script
(husky-env.sh): insert a line such as "# shellcheck shell=bash" immediately
after the existing SPDX header comments so static analysis and editors know the
intended shell; you may also include an editor-modeline like "# -*- shell: bash
-*-" if desired for extra clarity.
.husky/pre-commit (1)

17-22: Minor inconsistency in npx invocation.

Line 14 uses npx --no -- lint-staged but line 21 uses npx vitest run without --no --. Consider making the style consistent. The --no flag prevents npx from prompting to install the package if it's not found, which provides clearer failure behavior.

📝 Proposed fix
 else
-  npx vitest run
+  npx --no -- vitest run
 fi
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.husky/pre-commit around lines 17 - 22, The pre-commit script is
inconsistent: it uses `npx --no -- lint-staged` earlier but falls back to `npx
vitest run` in the vitest branch; update the vitest invocation to use the same
no-prompt style by replacing `npx vitest run` with `npx --no -- vitest run` (or
alternatively remove `--no --` from the lint-staged invocation if you prefer
prompting), ensuring you modify the branch that currently calls `npx vitest run`
so both invocations follow the same `--no --` convention.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In @.husky/pre-commit:
- Around line 17-22: The pre-commit script is inconsistent: it uses `npx --no --
lint-staged` earlier but falls back to `npx vitest run` in the vitest branch;
update the vitest invocation to use the same no-prompt style by replacing `npx
vitest run` with `npx --no -- vitest run` (or alternatively remove `--no --`
from the lint-staged invocation if you prefer prompting), ensuring you modify
the branch that currently calls `npx vitest run` so both invocations follow the
same `--no --` convention.

In @.husky/pre-push:
- Around line 31-55: Extract the duplicated logic that computes FROM and runs
hooks into a reusable shell function (e.g., run_outgoing_commits or
compute_from_and_run) and call it for both prek and pre-commit branches; the
function should accept the command name ("prek run" or "pre-commit run") or the
full command to execute, compute FROM using git merge-base HEAD '@{u}' fallback
to HEAD~1 or --all-files, and then invoke the provided command with --from-ref
"$FROM" --to-ref HEAD (or --all-files) so you remove the repeated blocks that
currently call prek run and pre-commit run and reuse the single helper instead.

In `@scripts/husky-env.sh`:
- Around line 1-2: Add a shell directive comment to help shellcheck and document
the expected shell for this sourced script (husky-env.sh): insert a line such as
"# shellcheck shell=bash" immediately after the existing SPDX header comments so
static analysis and editors know the intended shell; you may also include an
editor-modeline like "# -*- shell: bash -*-" if desired for extra clarity.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 393b47e0-9f21-4bfa-aca5-5cd79c2da7ac

📥 Commits

Reviewing files that changed from the base of the PR and between 42939f0 and d229a54.

📒 Files selected for processing (7)
  • .husky/commit-msg
  • .husky/pre-commit
  • .husky/pre-push
  • .pre-commit-config.yaml
  • CONTRIBUTING.md
  • scripts/husky-env.sh
  • scripts/install-openshell.sh

@cv cv merged commit d37a09f into NVIDIA:main Mar 22, 2026
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants