improved runtime of pre-commit significantly #1534
Conversation
WalkthroughReplaces the single-step pre-commit script with a parallel, file-type-aware pipeline and adds Husky helper files; minor comment/text edits in two scripts. Introduces conditional execution, unified wait-and-check for steps, aggregated secret checks, early exit when nothing is staged, and improved status reporting. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.husky/pre-commit (3)
228-231: Minor: Preferprintfoverechofor null-delimited data.
echomay add a trailing newline that could affect xargs processing. Consider usingprintf '%s'for more predictable behavior with null-delimited strings.- FILES_TO_CHECK=$(git diff --cached --name-only --diff-filter=ACM -z) - FILES_COUNT=$(echo "$FILES_TO_CHECK" | tr '\0' '\n' | wc -l | tr -d ' ') + FILES_TO_CHECK=$(git diff --cached --name-only --diff-filter=ACM -z) + FILES_COUNT=$(printf '%s' "$FILES_TO_CHECK" | tr '\0' '\n' | grep -c . || echo 0)Alternatively, pipe the git command directly:
FILES_COUNT=$(git diff --cached --name-only --diff-filter=ACM | wc -l | tr -d ' ')
252-255: Sameprintfconsideration for xargs input.For consistency with null-delimited data handling:
- echo "$FILES_TO_CHECK" | xargs -0 -n 1 -P "$PARALLEL_JOBS" sh "$TEMP_SCRIPT" > "${SECRET_CHECK_OUTPUT}.results" 2>&1 + printf '%s' "$FILES_TO_CHECK" | xargs -0 -n 1 -P "$PARALLEL_JOBS" sh "$TEMP_SCRIPT" > "${SECRET_CHECK_OUTPUT}.results" 2>&1
374-389: Potential duplicate warning output.The subshell (lines 269-276) already prints warning messages to
SECRET_CHECK_OUTPUT. Then lines 375-386 print additional messages based on the same patterns. For the private key case (exit code 0), this could result in duplicate warnings.Consider removing the redundant message printing in the subshell since the main script handles output display:
- elif echo "$SECRET_RESULT" | grep -q "Potential private key found"; then - printf '\033[36m%s\033[0m\n' "Warning: Potential Ethereum private keys found" - printf '\033[91m%s\033[0m\n' "Check each match carefully before pushing to GitHub" - EXIT_CODE=0 # Warning only - fi + fiOr alternatively, in the main script, skip the secondary warning print since
SECRET_RESULTalready contains the formatted message.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.husky/pre-commit(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1256
File: deployments/zksync.diamond.json:81-87
Timestamp: 2025-07-04T08:59:08.108Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1334
File: deployments/mainnet.json:54-54
Timestamp: 2025-08-26T02:20:52.515Z
Learning: For deployment PRs in the lifinance/contracts repository, carefully verify the specific scope mentioned in the PR title and description before suggesting updates to other networks. Not all deployments are cross-network updates - some are targeted to specific chains only.
Learnt from: mirooon
Repo: lifinance/contracts PR: 1283
File: deployments/ronin.diamond.json:65-68
Timestamp: 2025-08-07T10:20:01.383Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) and deployment log files have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1343
File: .husky/pre-commit:1-3
Timestamp: 2025-08-29T05:16:40.067Z
Learning: In .husky/pre-commit, user 0xDEnYO prefers to keep the existing POSIX sh shebang (#!/usr/bin/env sh) even when the script uses bash-only features like arrays and [[ ]] syntax, prioritizing working code stability over strict shell compliance when the current implementation functions correctly in their environment.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1196
File: script/helperFunctions.sh:1447-1462
Timestamp: 2025-06-19T06:23:47.848Z
Learning: 0xDEnYO prefers to keep eval usage in local bash scripts when the security risk is acceptable in their controlled environment, prioritizing simplicity over security hardening for local tooling.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1266
File: script/deploy/safe/execute-pending-timelock-tx.ts:627-628
Timestamp: 2025-07-17T04:21:26.825Z
Learning: In the lifinance/contracts repository, 0xDEnYO prefers to keep '0x0' as a fallback address in gas estimation calls rather than throwing errors when the wallet account address is not available, prioritizing code simplicity over strict validation.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1325
File: script/tasks/diamondSyncDEXs.sh:116-116
Timestamp: 2025-08-27T08:45:59.606Z
Learning: In script/tasks/diamondSyncDEXs.sh, user 0xDEnYO has chosen to selectively apply ShellCheck fixes, keeping array assignments using $() construct and other patterns as-is in their controlled deployment environment, prioritizing functionality over strict ShellCheck compliance.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1212
File: .github/workflows/convertForkedPRsToInternal.yml:81-106
Timestamp: 2025-07-16T06:18:02.682Z
Learning: 0xDEnYO prefers to use printf "%q" for shell escaping in GitHub workflows to increase security and protection from potential injections, even when it might cause formatting issues, prioritizing security over convenience.
📚 Learning: 2025-08-07T10:20:01.383Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1283
File: deployments/ronin.diamond.json:65-68
Timestamp: 2025-08-07T10:20:01.383Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) and deployment log files have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Applied to files:
.husky/pre-commit
📚 Learning: 2025-07-04T08:59:08.108Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1256
File: deployments/zksync.diamond.json:81-87
Timestamp: 2025-07-04T08:59:08.108Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Applied to files:
.husky/pre-commit
📚 Learning: 2025-08-29T05:16:40.067Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1343
File: .husky/pre-commit:1-3
Timestamp: 2025-08-29T05:16:40.067Z
Learning: In .husky/pre-commit, user 0xDEnYO prefers to keep the existing POSIX sh shebang (#!/usr/bin/env sh) even when the script uses bash-only features like arrays and [[ ]] syntax, prioritizing working code stability over strict shell compliance when the current implementation functions correctly in their environment.
Applied to files:
.husky/pre-commit
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: run-unit-tests
- GitHub Check: run-unit-tests
🔇 Additional comments (12)
.husky/pre-commit (12)
4-9: LGTM! Early exit optimization.The early exit check when no files are staged is a good optimization that avoids unnecessary work.
11-18: File type detection looks correct.The single-pass detection using
grep -qEwith short-circuit evaluation is efficient. Note that.jsxfiles are not included—this appears intentional if the project doesn't use JSX without TypeScript.
19-33: LGTM! Proper temp directory setup with cleanup.The trap correctly handles cleanup on EXIT, INT, and TERM signals. The temp file paths are well-organized for parallel execution.
34-52: Good helper functions for readable output.The colored status indicators (
✓,⊘,✗) improve user experience. The print functions are well-structured.
54-80: Output filtering logic is sound.The
|| truepattern correctly prevents grep from returning non-zero when no matches are found. Thehead -20limit on tsc output (line 74) may truncate useful error context, but is reasonable for pre-commit brevity.
82-111: Robust wait-and-check mechanism.The pattern of storing exit codes in files and reading them back is the correct approach for capturing exit status from backgrounded subshells. The fallback to
echo "1"on read failure is a safe default.
150-152: Excellent race condition prevention.Using
git show ":$FILE"to read staged content instead of the working directory file is the correct approach—it prevents race conditions when lint-staged modifies files concurrently.
283-316: Well-structured parallel execution.The pattern of running forge and lint-staged in parallel with proper cleanup on failure is correct. The subshell pattern
(cmd; echo $? > file) &correctly captures exit codes for backgrounded processes.
318-332: LGTM! Correct sequential dependency.TypeChain correctly runs after forge build completes, respecting the dependency that typechain needs compiled artifacts. The summary extraction provides good user feedback.
334-349: Verify: TypeScript-only changes skip type checking.The condition on line 335 requires both TS/JS files AND Solidity files to be changed. This means commits with only TypeScript changes won't run
tsc.If this is intentional for performance (avoiding tsc when typechain types haven't changed), consider adding a comment explaining the rationale. Otherwise, you may want to run tsc whenever
HAS_TS_JS_FILES = yes.-if [ "$HAS_TS_JS_FILES" = "yes" ] && [ "$HAS_SOL_FILES" = "yes" ]; then +# Only run tsc when Solidity files changed (typechain types may have changed) +# TS-only changes rely on lint-staged for basic validation +if [ "$HAS_TS_JS_FILES" = "yes" ] && [ "$HAS_SOL_FILES" = "yes" ]; then
351-358: LGTM! Proper lint-staged completion handling.The wait occurs after other sequential tasks, allowing lint-staged to run in parallel for maximum efficiency while still ensuring it completes before the final status.
391-392: LGTM! Clean completion.The final success message and exit 0 provide clear feedback that all checks passed.
…tracts into improve-pre-commit-runtime
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
.husky/_/.gitignore(1 hunks).husky/_/husky.sh(1 hunks).husky/pre-commit(3 hunks)script/deploy/healthCheck.ts(1 hunks)script/playgroundHelpers.sh(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- script/deploy/healthCheck.ts
🧰 Additional context used
📓 Path-based instructions (1)
script/**/*.sh
📄 CodeRabbit inference engine (conventions.md)
script/**/*.sh: Bash scripts must begin with #!/bin/bash, use modular functions with clear sections (Logging, Error handling and logging, Deployment functions), follow DRY principle using helper files, use consistent indentation and naming, include proper comments and documentation
Bash scripts must use helper functions for logging (echoDebug, error, warning, success), validate inputs and environment early, check function exit status with checkFailure, use set -e where appropriate, and provide automated retry mechanisms for RPC issues
Files:
script/playgroundHelpers.sh
🧠 Learnings (6)
📓 Common learnings
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1256
File: deployments/zksync.diamond.json:81-87
Timestamp: 2025-07-04T08:59:08.108Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Learnt from: mirooon
Repo: lifinance/contracts PR: 1283
File: deployments/ronin.diamond.json:65-68
Timestamp: 2025-08-07T10:20:01.383Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) and deployment log files have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1334
File: deployments/mainnet.json:54-54
Timestamp: 2025-08-26T02:20:52.515Z
Learning: For deployment PRs in the lifinance/contracts repository, carefully verify the specific scope mentioned in the PR title and description before suggesting updates to other networks. Not all deployments are cross-network updates - some are targeted to specific chains only.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1343
File: .husky/pre-commit:1-3
Timestamp: 2025-08-29T05:16:40.067Z
Learning: In .husky/pre-commit, user 0xDEnYO prefers to keep the existing POSIX sh shebang (#!/usr/bin/env sh) even when the script uses bash-only features like arrays and [[ ]] syntax, prioritizing working code stability over strict shell compliance when the current implementation functions correctly in their environment.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1196
File: script/helperFunctions.sh:1447-1462
Timestamp: 2025-06-19T06:23:47.848Z
Learning: 0xDEnYO prefers to keep eval usage in local bash scripts when the security risk is acceptable in their controlled environment, prioritizing simplicity over security hardening for local tooling.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1266
File: script/deploy/safe/execute-pending-timelock-tx.ts:627-628
Timestamp: 2025-07-17T04:21:26.825Z
Learning: In the lifinance/contracts repository, 0xDEnYO prefers to keep '0x0' as a fallback address in gas estimation calls rather than throwing errors when the wallet account address is not available, prioritizing code simplicity over strict validation.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1325
File: script/tasks/diamondSyncDEXs.sh:116-116
Timestamp: 2025-08-27T08:45:59.606Z
Learning: In script/tasks/diamondSyncDEXs.sh, user 0xDEnYO has chosen to selectively apply ShellCheck fixes, keeping array assignments using $() construct and other patterns as-is in their controlled deployment environment, prioritizing functionality over strict ShellCheck compliance.
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1212
File: .github/workflows/convertForkedPRsToInternal.yml:81-106
Timestamp: 2025-07-16T06:18:02.682Z
Learning: 0xDEnYO prefers to use printf "%q" for shell escaping in GitHub workflows to increase security and protection from potential injections, even when it might cause formatting issues, prioritizing security over convenience.
📚 Learning: 2025-08-29T05:16:40.067Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1343
File: .husky/pre-commit:1-3
Timestamp: 2025-08-29T05:16:40.067Z
Learning: In .husky/pre-commit, user 0xDEnYO prefers to keep the existing POSIX sh shebang (#!/usr/bin/env sh) even when the script uses bash-only features like arrays and [[ ]] syntax, prioritizing working code stability over strict shell compliance when the current implementation functions correctly in their environment.
Applied to files:
.husky/_/husky.sh.husky/_/.gitignore.husky/pre-commit
📚 Learning: 2025-07-04T08:59:08.108Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 1256
File: deployments/zksync.diamond.json:81-87
Timestamp: 2025-07-04T08:59:08.108Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Applied to files:
script/playgroundHelpers.sh.husky/pre-commit
📚 Learning: 2025-08-07T10:20:01.383Z
Learnt from: mirooon
Repo: lifinance/contracts PR: 1283
File: deployments/ronin.diamond.json:65-68
Timestamp: 2025-08-07T10:20:01.383Z
Learning: When analyzing deployment PRs in the lifinance/contracts repository, carefully verify that target state configuration files (like script/deploy/_targetState.json) and deployment log files have been updated before flagging missing entries. The AI summary section should be consulted to understand all file changes, as manual searches might miss entries due to formatting differences or search limitations.
Applied to files:
script/playgroundHelpers.sh.husky/pre-commit
📚 Learning: 2024-11-26T01:18:52.125Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 782
File: script/deploy/deploySingleContract.sh:231-231
Timestamp: 2024-11-26T01:18:52.125Z
Learning: In the `deploySingleContract.sh` script, environment variable assignments preceding the `forge script` command (e.g., `DEPLOYSALT=$DEPLOYSALT ... forge script ...`) are acceptable and function correctly, even when variable expansions like `$DIAMOND_TYPE` and `$NETWORK` are used later in the same command.
Applied to files:
.husky/pre-commit
📚 Learning: 2024-12-05T03:41:16.360Z
Learnt from: 0xDEnYO
Repo: lifinance/contracts PR: 894
File: .github/workflows/diamondEmergencyPause.yml:68-68
Timestamp: 2024-12-05T03:41:16.360Z
Learning: Before reporting missing GitHub secrets in the lifinance/contracts repository, conclusively verify that the secret is absent to avoid false alerts.
Applied to files:
.husky/pre-commit
🧬 Code graph analysis (1)
script/playgroundHelpers.sh (1)
script/helperFunctions.sh (1)
error(3117-3119)
🪛 Shellcheck (0.11.0)
.husky/_/husky.sh
[warning] 9-9: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 19-19: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: run-unit-tests
🔇 Additional comments (6)
.husky/_/.gitignore (1)
1-3: Good Husky hygiene: ignore everything except the wrapper.
Keeps onlyhusky.sh(and this.gitignore) tracked, avoiding accidental commits of generated/temporary Husky files.script/playgroundHelpers.sh (1)
720-736: Safer docs: placeholder examples instead of real tx hash / RPC.
This is purely messaging and reduces accidental reuse/leaking of real values..husky/pre-commit (3)
1-14: Shebang change to bash is fine, but please verify Husky executes it as bash everywhere.
This hook now relies on bash features (arrays,[[ ... ]], process substitution). Based on prior learnings about preferring#!/usr/bin/env shin this repo, double-check this doesn’t break anyone’s local hook execution path.
61-295: Secret-check approach looks solid (staged-content scan +.envkey-only reporting).
Scanning staged blobs viagit show ":$FILE"avoids false negatives from unstaged working tree changes, and the output doesn’t echo secret values.Also applies to: 371-399
15-22: The lint-staged and tsc configurations are correct; no coverage regression exists.
- Lint-staged runs on commits with any lintable files (
.solor.(ts|js)), not just when both types change. TheHAS_TYPE_FILESgate is inclusive, not restrictive.tsc-filesis already part of the lint-staged config for*.{ts,js}files, so TS-only commits receive type checking via lint-staged.- JSON/MD/YAML files are not configured in lint-staged, so commits affecting only those file types skipping lint-staged is expected, not a regression.
- The separate tsc invocation (lines 347–362) only runs when Solidity files also changed; this is redundant with lint-staged's tsc-files but not a gap.
.husky/_/husky.sh (1)
1-37: No issues found. The code works correctly across environments, and bothbasename --and string-based exit code comparisons function as intended. Since this file is part of the Husky framework and is already working in the repository's current setup, stylistic portability improvements are not necessary.
| # Early exit: Check if there are any staged files | ||
| STAGED_FILES=$(git diff --cached --name-only --diff-filter=ACM) | ||
| if [ -z "$STAGED_FILES" ]; then | ||
| echo "No files staged for commit. Skipping pre-commit checks." | ||
| exit 0 | ||
| fi | ||
|
|
||
| echo "TypeChain generation completed successfully!" | ||
| echo "" | ||
| GIT_ROOT=$(git rev-parse --show-toplevel) | ||
| cd "$GIT_ROOT" || exit 1 | ||
|
|
||
| # Run TypeScript compilation to catch any type errors from outdated typings | ||
| echo "Running TypeScript compilation to check for type errors..." | ||
| bunx tsc-files --noEmit | ||
| # Determine what types of files changed for conditional execution (optimized single pass) | ||
| HAS_SOL_FILES="no" | ||
| HAS_TS_JS_FILES="no" | ||
| HAS_TYPE_FILES="no" | ||
|
|
||
| if [ $? -ne 0 ]; then | ||
| printf '\n%s\n' "TypeScript compilation failed. This may indicate outdated TypeChain types." | ||
| printf '%s\n\n' "Please run 'bun typechain' manually and fix any type errors before committing." | ||
| exit 1 | ||
| fi | ||
| echo "$STAGED_FILES" | grep -qE '\.sol$' && HAS_SOL_FILES="yes" && HAS_TYPE_FILES="yes" | ||
| echo "$STAGED_FILES" | grep -qE '\.(ts|js|tsx)$' && HAS_TS_JS_FILES="yes" && HAS_TYPE_FILES="yes" | ||
|
|
There was a problem hiding this comment.
diff-filter=ACM skips renames; consider including R.
Renamed files can still carry secrets / need formatting; they’ll be missed by both the early staged list and secret scanning.
-STAGED_FILES=$(git diff --cached --name-only --diff-filter=ACM)
+STAGED_FILES=$(git diff --cached --name-only --diff-filter=ACMR)And likewise in run_secret_checks:
- done < <(git diff --cached --name-only --diff-filter=ACM -z)
+ done < <(git diff --cached --name-only --diff-filter=ACMR -z)Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In .husky/pre-commit around lines 5 to 22 the staged-files command uses git diff
--cached --name-only --diff-filter=ACM which omits renamed files; update the
diff-filter to include R (e.g., ACMR) so renamed files are returned and scanned,
and apply the same change inside run_secret_checks where git lists staged files
so renames are not skipped by secret scans or formatting checks.
| # Create temp directory for parallel execution | ||
| TEMP_DIR=$(mktemp -d) | ||
| trap "rm -rf '$TEMP_DIR' 2>/dev/null" EXIT INT TERM | ||
|
|
||
| echo "" | ||
| echo "Running 'bun lint-staged' now:" | ||
| bun lint-staged | ||
|
|
||
| echo "" | ||
| echo "" | ||
| echo "now checking for .env secrets and private keys accidentally being committed to Github" | ||
| echo " > any 64-byte hex string will be identified as potential private key" | ||
| echo " > disable false positive matches by commenting '[pre-commit-checker: not a secret]' in same line or line above" | ||
| echo " > execution might take a while depending on the size of your git diff " | ||
| echo " > logs will only be shown once task is completed" | ||
| echo "" | ||
| FORGE_OUTPUT="$TEMP_DIR/forge.out" | ||
| FORGE_EXIT="$TEMP_DIR/forge.exit" | ||
| TYPECHAIN_OUTPUT="$TEMP_DIR/typechain.out" | ||
| TYPECHAIN_EXIT="$TEMP_DIR/typechain.exit" | ||
| TSC_OUTPUT="$TEMP_DIR/tsc.out" | ||
| TSC_EXIT="$TEMP_DIR/tsc.exit" | ||
| LINT_STAGED_OUTPUT="$TEMP_DIR/lint-staged.out" | ||
| LINT_STAGED_EXIT="$TEMP_DIR/lint-staged.exit" | ||
|
|
There was a problem hiding this comment.
mktemp -d is not portable on macOS; add fallback.
This is a common “works on Linux, fails on mac” footgun for local hooks.
-TEMP_DIR=$(mktemp -d)
+TEMP_DIR="$(mktemp -d 2>/dev/null || mktemp -d -t husky-precommit)"🤖 Prompt for AI Agents
.husky/pre-commit around lines 23 to 35: the script uses mktemp -d which fails
on macOS; modify the TEMP_DIR creation to try the portable form and fall back
for macOS: attempt TEMP_DIR=$(mktemp -d 2>/dev/null) and if that fails, use
TEMP_DIR=$(mktemp -d -t precommit) (or create a fallback with mkdir and a random
suffix), keep the existing trap cleanup, and ensure all subsequent TEMP_DIR
usages remain unchanged.
| printAdvise "abort" | ||
| echo "" | ||
| # TypeScript compilation check (only if both TS/JS files changed AND typechain was regenerated) | ||
| if [ "$HAS_TS_JS_FILES" = "yes" ] && [ "$HAS_SOL_FILES" = "yes" ]; then |
There was a problem hiding this comment.
I think here we don't have to have condition [ "$HAS_SOL_FILES" = "yes" ] as we only need to run tsc files on ts/js files
| exit 0 | ||
| } | ||
| print_status "info" "Checking for secrets and private keys..." | ||
| run_secret_checks |
There was a problem hiding this comment.
I would run it before all heavy work
|
|
||
| # process all files in git diff and search for secrets | ||
| local RESULT=$(processGitDiff) | ||
| if [ "$HAS_TYPE_FILES" = "yes" ]; then |
There was a problem hiding this comment.
I’m not sure we gain much from this and it adds extra complexity. lint-staged is already scoped in package.json to the file extensions it should scan. If we introduce new linting rules for additional extensions, we’d have to remember to update both package.json and the pre-commit script
Why did I implement it this way?
Sometimes the pre-commit check was taking forever, especially on larger git diffs.
This PR introduces some optimizations and parallelizes tasks more efficiently which significantly reduces the runtime of the checks.
This is vibe-coded, please review carefully.
Checklist before requesting a review
Checklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)