Skip to content

feat(tooling): SPDX --fix, hook ordering, and strict shellcheck fixes #670

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
HagegeR wants to merge 7 commits into
base: main
Choose a base branch
from
Open

feat(tooling): SPDX --fix, hook ordering, and strict shellcheck fixes #670

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
dc2142d
feat(scripts): add SPDX --fix and exclude blueprint __init__.py files
HagegeR Mar 22, 2026
3f61059
chore: SPDX hook at priority 4 with --fix; lint-staged uses --fix
HagegeR Mar 22, 2026
557d8ac
chore: add SPDX headers to shell scripts missing them
HagegeR Mar 22, 2026
7f02c42
chore(pre-commit): run shellcheck with default rules
HagegeR Mar 22, 2026
a455473
fix(scripts): declare bash for shellcheck in husky-env
HagegeR Mar 22, 2026
ff76418
chore(scripts): normalize redirection in test-inference helpers
HagegeR Mar 22, 2026
1107926
fix(e2e): satisfy shellcheck in onboard and full e2e tests
HagegeR Mar 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#
# Priority groups (prek: same priority may run in parallel; stock pre-commit ignores priority):
# 0 — General file fixers (whitespace, EOF, line endings)
# 4 — SPDX header insertion (--fix)
# 5 — Shell / Python / TS formatters (shfmt, ruff format, prettier)
# 6 — Fixes that should follow formatters (ruff check --fix, eslint --fix)
# 10 — Linters and read-only checks
Expand All @@ -38,6 +39,18 @@ repos:
args: ["--fix=lf"]
priority: 0

# ── Priority 4: SPDX headers (insert if missing; runs before language formatters) ──
- repo: local
hooks:
- id: spdx-headers
name: SPDX license headers (insert if missing)
entry: bash scripts/check-spdx-headers.sh --fix
language: system
files: ^(nemoclaw/src/.*\.ts|nemoclaw-blueprint/.*\.py|.*\.sh|\.husky/.*)$
exclude: ^nemoclaw-blueprint/.*__init__\.py$
pass_filenames: true
priority: 4

# ── Priority 5: formatters ────────────────────────────────────────────────
- repo: https://github.com/scop/pre-commit-shfmt
rev: v3.12.0-2
Expand Down Expand Up @@ -111,9 +124,6 @@ repos:
rev: v0.11.0.1
hooks:
- id: shellcheck
args:
- --severity=warning
- --exclude=SC1091
priority: 10

- repo: https://github.com/hadolint/hadolint
Expand All @@ -122,16 +132,6 @@ repos:
- id: hadolint
priority: 10

- repo: local
hooks:
- id: spdx-headers
name: SPDX license headers
entry: bash scripts/check-spdx-headers.sh
language: system
files: ^(nemoclaw/src/.*\.ts|nemoclaw-blueprint/.*\.py|.*\.sh|\.husky/.*)$
pass_filenames: true
priority: 10

- repo: https://github.com/gitleaks/gitleaks
rev: v8.30.1
hooks:
Expand Down
11 changes: 7 additions & 4 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,20 +33,23 @@
},
"lint-staged": {
"nemoclaw/src/**/*.ts": [
"bash scripts/check-spdx-headers.sh",
"bash scripts/check-spdx-headers.sh --fix",
"sh -c 'cd nemoclaw && npx eslint --fix \"$@\"' _",
"sh -c 'cd nemoclaw && npx prettier --write \"$@\"' _"
],
"nemoclaw-blueprint/**/*.py": [
"bash scripts/check-spdx-headers.sh",
"bash scripts/check-spdx-headers.sh --fix",
"uvx ruff check --fix",
Comment on lines 40 to 42
Copy link

@coderabbitai coderabbitai bot Mar 22, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

find nemoclaw-blueprint -type f -name '__init__.py' | sort

Repository: NVIDIA/NemoClaw

Length of output: 102

🏁 Script executed:

cat -n .pre-commit-config.yaml | head -100

Repository: NVIDIA/NemoClaw

Length of output: 4520

🏁 Script executed:

cat -n package.json | sed -n '35,50p'

Repository: NVIDIA/NemoClaw

Length of output: 719

Keep blueprint __init__.py files out of the lint-staged fixer.

.pre-commit-config.yaml explicitly excludes nemoclaw-blueprint/**/__init__.py from the SPDX headers fixer (line 50), but the lint-staged glob nemoclaw-blueprint/**/*.py (lines 40–43) sends every staged blueprint __init__.py file to check-spdx-headers.sh --fix. At least one such file exists (nemoclaw-blueprint/orchestrator/__init__.py). This creates inconsistent behavior between the two hooks and should be excluded from lint-staged as well.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@package.json` around lines 40 - 42, The lint-staged glob
"nemoclaw-blueprint/**/*.py" is including blueprint __init__.py files (e.g.
nemoclaw-blueprint/orchestrator/__init__.py) while .pre-commit-config.yaml
explicitly excludes them; update the lint-staged entry for
"nemoclaw-blueprint/**/*.py" to also exclude __init__.py files (for example by
adding a negative glob such as "nemoclaw-blueprint/**/__init__.py" prefixed with
"!" or an equivalent exclusion) so the SPDX header fixer and lint-staged
behavior match.

"uvx ruff format"
],
"**/*.sh": [
"bash scripts/check-spdx-headers.sh",
"bash scripts/check-spdx-headers.sh --fix",
"npx shellcheck"
],
".husky/{pre-commit,pre-push,commit-msg}": "npx shellcheck"
".husky/{pre-commit,pre-push,commit-msg}": [
"bash scripts/check-spdx-headers.sh --fix",
"npx shellcheck"
]
},
"devDependencies": {
"@commitlint/cli": "^20.5.0",
Expand Down
96 changes: 85 additions & 11 deletions scripts/check-spdx-headers.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,99 @@
# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
# SPDX-License-Identifier: Apache-2.0

# Checks that all given files contain the required SPDX license headers.
# Usage: check-spdx-headers.sh file1 file2 ...
# SPDX header check (and optional auto-insert), similar in spirit to DOCA's check_doca_license.py --fix.
#
# Usage:
# check-spdx-headers.sh FILE... # fail if header missing
# check-spdx-headers.sh --fix FILE... # insert NVIDIA Apache-2.0 SPDX block after shebang (if any)

set -euo pipefail

COPYRIGHT="SPDX-FileCopyrightText: Copyright (c)"
LICENSE="SPDX-License-Identifier: Apache-2.0"
COPYRIGHT_SUBSTR="SPDX-FileCopyrightText: Copyright (c)"
LICENSE_SUBSTR="SPDX-License-Identifier: Apache-2.0"

usage() {
echo "Usage: $0 [--fix] FILE..." >&2
exit 2
}

FIX=false
if [[ "${1:-}" == "--fix" ]]; then
FIX=true
shift
fi

[[ $# -gt 0 ]] || usage

has_spdx() {
local file=$1
local head
head=$(head -n 16 -- "$file" 2>/dev/null) || return 1
grep -Fq "$COPYRIGHT_SUBSTR" <<<"$head" || return 1
grep -Fq "$LICENSE_SUBSTR" <<<"$head" || return 1
return 0
}

comment_style_for() {
local base
base=$(basename "$1")
case "$base" in
Dockerfile | *.dockerfile | *.Dockerfile) echo "#" ;;
*.ts | *.tsx | *.js | *.mjs | *.cjs) echo "//" ;;
*) echo "#" ;;
esac
}

spdx_block() {
local style=$1
local year
year=$(date +%Y)
if [[ "$style" == "//" ]]; then
printf '// SPDX-FileCopyrightText: Copyright (c) %s NVIDIA CORPORATION & AFFILIATES. All rights reserved.\n' "$year"
printf '// SPDX-License-Identifier: Apache-2.0\n'
else
printf '# SPDX-FileCopyrightText: Copyright (c) %s NVIDIA CORPORATION & AFFILIATES. All rights reserved.\n' "$year"
printf '# SPDX-License-Identifier: Apache-2.0\n'
fi
}

insert_spdx() {
local file=$1
local style
style=$(comment_style_for "$file")
local tmp
tmp="$(mktemp "${TMPDIR:-/tmp}/nemoclaw-spdx.XXXXXX")"
{
IFS= read -r first || true
if [[ "$first" == '#!'* ]]; then
printf '%s\n' "$first"
spdx_block "$style"
printf '\n'
cat
else
spdx_block "$style"
printf '\n'
if [[ -n "${first:-}" ]]; then
printf '%s\n' "$first"
fi
cat
fi
} <"$file" >"$tmp" && mv "$tmp" "$file"
Comment on lines +65 to +82
Copy link

@coderabbitai coderabbitai bot Mar 22, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT

src="$tmpdir/pre-commit"
printf '#!/usr/bin/env bash\necho ok\n' >"$src"
chmod 755 "$src"

tmp="$(mktemp "${TMPDIR:-/tmp}/nemoclaw-spdx.XXXXXX")"
printf '#!/usr/bin/env bash\n# SPDX-License-Identifier: Apache-2.0\n\necho ok\n' >"$tmp"
mv "$tmp" "$src"

stat -c '%a %n' "$src" 2>/dev/null || stat -f '%Lp %N' "$src"

Repository: NVIDIA/NemoClaw

Length of output: 104

🏁 Script executed:

#!/bin/bash
# First, find the script file
find . -name "check-spdx-headers.sh" -type f

Repository: NVIDIA/NemoClaw

Length of output: 90

🏁 Script executed:

#!/bin/bash
# Test mktemp + mv behavior without stat (which isn't available)
# Create a test directory
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT

# Create original file with executable permissions
src="$tmpdir/test-script"
printf '#!/usr/bin/env bash\necho ok\n' >"$src"
chmod 755 "$src"

echo "Original file permissions (using ls -l):"
ls -l "$src"

# Simulate what mktemp does
tmp="$(mktemp "${TMPDIR:-/tmp}/test.XXXXXX")"
printf '#!/usr/bin/env bash\n# Header\necho ok\n' >"$tmp"

echo "Temp file permissions after creation (using ls -l):"
ls -l "$tmp"

# Now do the mv
mv "$tmp" "$src"

echo "Original file permissions after mv (using ls -l):"
ls -l "$src"

# Clean up
rm -f "$src"

Repository: NVIDIA/NemoClaw

Length of output: 423

🏁 Script executed:

cat ./scripts/check-spdx-headers.sh

Repository: NVIDIA/NemoClaw

Length of output: 2527

🏁 Script executed:

# Check if this script is used on executable files/hooks
grep -r "check-spdx-headers.sh" --include="*.md" --include="*.yaml" --include="*.yml" --include="*.json"

Repository: NVIDIA/NemoClaw

Length of output: 390

Preserve the original file permissions when rewriting scripts with mktemp and mv.

The insert_spdx function uses mktemp to create a temporary file (which defaults to mode 0600) and then overwrites the original file with mv "$tmp" "$file". Since mv preserves the source file's permissions, any executable script or hook processed by --fix will lose its executable bit (+x), breaking subsequent hook execution and failing shebang/executable validation.

Suggested fix 
 insert_spdx() {
   local file=$1
   local style
   style=$(comment_style_for "$file")
   local tmp
+  local mode
   tmp="$(mktemp "${TMPDIR:-/tmp}/nemoclaw-spdx.XXXXXX")"
+  mode="$(stat -c '%a' "$file" 2>/dev/null || stat -f '%Lp' "$file")"
   {
     IFS= read -r first || true
     if [[ "$first" == '#!'* ]]; then
       printf '%s\n' "$first"
       spdx_block "$style"
       printf '\n'
       cat
     else
       spdx_block "$style"
       printf '\n'
       if [[ -n "${first:-}" ]]; then
         printf '%s\n' "$first"
       fi
       cat
     fi
-  } <"$file" >"$tmp" && mv "$tmp" "$file"
+  } <"$file" >"$tmp" && chmod "$mode" "$tmp" && mv "$tmp" "$file"
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
local tmp
tmp="$(mktemp "${TMPDIR:-/tmp}/nemoclaw-spdx.XXXXXX")"
{
IFS= read -r first || true
if [[ "$first" == '#!'* ]]; then
printf '%s\n' "$first"
spdx_block "$style"
printf '\n'
cat
else
spdx_block "$style"
printf '\n'
if [[ -n "${first:-}" ]]; then
printf '%s\n' "$first"
fi
cat
fi
} <"$file" >"$tmp" && mv "$tmp" "$file"
local tmp
local mode
tmp="$(mktemp "${TMPDIR:-/tmp}/nemoclaw-spdx.XXXXXX")"
mode="$(stat -c '%a' "$file" 2>/dev/null || stat -f '%Lp' "$file")"
{
IFS= read -r first || true
if [[ "$first" == '#!'* ]]; then
printf '%s\n' "$first"
spdx_block "$style"
printf '\n'
cat
else
spdx_block "$style"
printf '\n'
if [[ -n "${first:-}" ]]; then
printf '%s\n' "$first"
fi
cat
fi
} <"$file" >"$tmp" && chmod "$mode" "$tmp" && mv "$tmp" "$file"
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@scripts/check-spdx-headers.sh` around lines 65 - 82, The temp-file rewrite
currently creates a 0600 file and then mv's it over the original, losing
executable bits; before mv "$tmp" "$file" set the temp file's mode to match the
original (e.g. run chmod --reference="$file" "$tmp" or use stat to get and apply
the original mode) inside the insert_spdx block that creates tmp so the original
file permissions (including +x) are preserved when moving the temp into place.

}

failed=0
for file in "$@"; do
file_head="$(head -n 5 -- "$file")"
if ! grep -Fq "$COPYRIGHT" <<< "$file_head"; then
echo "Missing SPDX-FileCopyrightText: $file"
failed=1
[[ -f "$file" ]] || continue
if has_spdx "$file"; then
continue
fi
if ! grep -Fq "$LICENSE" <<< "$file_head"; then
echo "Missing SPDX-License-Identifier: $file"
if [[ "$FIX" == true ]]; then
echo "Adding SPDX header: $file"
insert_spdx "$file"
else
echo "Missing SPDX-FileCopyrightText or SPDX-License-Identifier (first ~16 lines): $file"
failed=1
fi
done

exit $failed
exit "$failed"
1 change: 1 addition & 0 deletions scripts/husky-env.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# shellcheck shell=bash
# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
# SPDX-License-Identifier: Apache-2.0
#
Expand Down
5 changes: 4 additions & 1 deletion scripts/test-inference-local.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
#!/usr/bin/env bash
# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
# SPDX-License-Identifier: Apache-2.0

# Test inference.local routing through OpenShell provider (local vLLM)
echo '{"model":"nvidia/nemotron-3-nano-30b-a3b","messages":[{"role":"user","content":"say hello"}]}' > /tmp/req.json
echo '{"model":"nvidia/nemotron-3-nano-30b-a3b","messages":[{"role":"user","content":"say hello"}]}' >/tmp/req.json
curl -s https://inference.local/v1/chat/completions -H "Content-Type: application/json" -d @/tmp/req.json
5 changes: 4 additions & 1 deletion scripts/test-inference.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
#!/usr/bin/env bash
# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
# SPDX-License-Identifier: Apache-2.0

# Test inference.local routing through OpenShell provider
echo '{"model":"nvidia/nemotron-3-super-120b-a12b","messages":[{"role":"user","content":"say hello"}]}' > /tmp/req.json
echo '{"model":"nvidia/nemotron-3-super-120b-a12b","messages":[{"role":"user","content":"say hello"}]}' >/tmp/req.json
curl -s https://inference.local/v1/chat/completions -H "Content-Type: application/json" -d @/tmp/req.json
Loading