feat(devops): add Husky pre-commit hooks for linting and type checkin… - #676
feat(devops): add Husky pre-commit hooks for linting and type checkin…#676Venrable18 wants to merge 1 commit into
Conversation
|
@Venrable18 is attempting to deploy a commit to the ritik4ever's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds Husky/lint-staged pre-commit checks and related contributor documentation. Backend middleware, filtering, health, pagination, response, error, and logging code is reformatted without described runtime changes. ChangesPre-commit tooling and backend formatting
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
@Venrable18 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
backend/src/index.ts (1)
911-912: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the stray test comments.
These debug artifacts are unrelated to the formatting change and may confuse future maintainers.
Proposed cleanup
-// test -// test🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/index.ts` around lines 911 - 912, Remove the two stray "// test" comments from the affected code in index.ts, leaving the surrounding formatting change and implementation unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/src/index.ts`:
- Around line 134-136: Update the production setup in the app initialization
around apiKeyAuthMiddleware so authentication fails closed when API_KEYS is
missing. Require API_KEYS during production startup or ensure the middleware
rejects every request without configured keys, while preserving normal
authenticated behavior when keys are present.
- Around line 361-363: Update the health-check details in the surrounding
status-building logic so a configured config.sorobanRpcUrl is not described as
“Soroban RPC reachable” without an actual connectivity result. Use the existing
RPC reachability check or report only configuration status, preserving the
unconfigured message when no URL is provided.
In `@package.json`:
- Around line 43-48: Update the lint-staged compiler and Cargo clippy entries to
use filename-agnostic JavaScript functions or argument-ignoring wrapper scripts,
so matched filenames are not appended to `tsc` or `cargo clippy`; preserve the
existing backend, frontend, and contracts working-directory commands and strict
clippy warnings.
---
Nitpick comments:
In `@backend/src/index.ts`:
- Around line 911-912: Remove the two stray "// test" comments from the affected
code in index.ts, leaving the surrounding formatting change and implementation
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 929ccc79-c321-4bee-a9ea-fca971eb8c11
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
CONTRIBUTING.mdbackend/src/index.tspackage.json
| if (process.env.NODE_ENV === 'production') { | ||
| app.use(apiKeyAuthMiddleware); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Fail closed when production API keys are missing.
backend/src/middleware/apiKeyAuth.ts accepts any Bearer token when API_KEYS is empty. Since this file mounts that middleware in production, a deployment missing API_KEYS has no effective authentication. Require the variable during production startup or reject requests when it is absent.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/index.ts` around lines 134 - 136, Update the production setup in
the app initialization around apiKeyAuthMiddleware so authentication fails
closed when API_KEYS is missing. Require API_KEYS during production startup or
ensure the middleware rejects every request without configured keys, while
preserving normal authenticated behavior when keys are present.
| details: config.sorobanRpcUrl | ||
| ? 'Soroban RPC reachable' | ||
| : 'Soroban RPC URL not configured', |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Do not report a configured RPC as reachable.
This condition only checks whether config.sorobanRpcUrl exists; it performs no connectivity check. A configured but unavailable RPC will be reported as reachable, misleading health checks and monitoring.
Proposed minimal fix
- ? 'Soroban RPC reachable'
+ ? 'Soroban RPC configured'📝 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.
| details: config.sorobanRpcUrl | |
| ? 'Soroban RPC reachable' | |
| : 'Soroban RPC URL not configured', | |
| details: config.sorobanRpcUrl | |
| ? 'Soroban RPC configured' | |
| : 'Soroban RPC URL not configured', |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/index.ts` around lines 361 - 363, Update the health-check details
in the surrounding status-building logic so a configured config.sorobanRpcUrl is
not described as “Soroban RPC reachable” without an actual connectivity result.
Use the existing RPC reachability check or report only configuration status,
preserving the unconfigured message when no URL is provided.
| "cd backend && tsc --noEmit" | ||
| ], | ||
| "frontend/**/*.{ts,tsx}": [ | ||
| "cd frontend && eslint --fix", | ||
| "prettier --write", | ||
| "cd frontend && tsc --noEmit" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Run in a disposable worktree with representative staged files.
npx lint-staged --debug 2>&1 | tee /tmp/lint-staged-debug.log
grep -E 'tsc --noEmit|cargo clippy' /tmp/lint-staged-debug.logRepository: ritik4ever/stellar-goal-vault
Length of output: 159
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "package.json:"
if [ -f package.json ]; then
cat -n package.json | sed -n '1,100p'
else
echo "package.json not found"
fi
echo
echo "lint-staged config references:"
grep -RIn '"lint-staged"|"lintStaged"|lint-staged|lintStaged' package.json pnpm-lock.yaml package-lock.json yarn.lock 2>/dev/null | sed -n '1,160p' || true
echo
echo "TypeScript config files:"
fd -a 'tsconfig.*json$|tsconfig' . | sed 's#^\./##' | sort | head -80
echo
echo "Rust files:"
git ls-files | grep -E '(^|/)Cargo\.toml$|\.rs$' | head -100Repository: ritik4ever/stellar-goal-vault
Length of output: 2751
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
# Deterministic model of lint-staged string task command:
# each matched string command is appended with the list of filenames.
commands = [
"cd backend && tsc --noEmit",
"cd frontend && tsc --noEmit",
"cd contracts && cargo clippy -- -D warnings",
]
files = ["backend/src/foo.ts", "frontend/src/bar.tsx", "contracts/src/lib.rs"]
for cmd in commands:
full = f"{cmd} { ' '.join(files) }"
print(full)
PY
echo
echo "TypeScript configuration snippets:"
for f in backend/tsconfig.json frontend/tsconfig.json tsconfig.json frontend/tsconfig.node.json frontend/tsconfig.test.json backend/tsconfig.eslint.json; do
if [ -f "$f" ]; then
echo "--- $f"
python3 - <<PY "$f"
import json, sys
p=sys.argv[1]
try:
data=json.load(open(p))
except Exception as e:
print(f"error: {e}")
raise
if isinstance(data, dict):
for k in ["compilerOptions","extends","include","exclude","references"]:
if k in data:
print(k, "=>", data[k])
else:
print(data)
PY
fi
done
echo
echo "lockfile lint-staged version:"
for f in package-lock.json pnpm-lock.yaml yarn.lock; do
[ -f "$f" ] && { echo "--- $f"; grep -n "lint-staged" "$f" | sed -n '1,80p'; }
doneRepository: ritik4ever/stellar-goal-vault
Length of output: 1947
🌐 Web query:
lint-staged string tasks append staged file names lint-staged documentation
💡 Result:
By default, lint-staged automatically appends the list of matched staged files (passed as absolute paths) to the end of your configured command string [1][2][3]. For example, if you configure a task like this: "*.js": "prettier --write" And you have staged files named file-1.js and file-2.js, lint-staged will execute the following command: prettier --write /absolute/path/to/file-1.js /absolute/path/to/file-2.js [1][3][4] Key points to remember: 1. Automatic Appending: You do not need to specify where the file names go in a standard string command; lint-staged handles this for you [1][4]. If you include a path (e.g., "prettier --write."), lint-staged will append the staged files after that path, which may cause unexpected behavior [4]. 2. Function Syntax: When using the advanced JavaScript function syntax in your configuration, lint-staged does not automatically append the file names [1][5]. In these cases, you are responsible for manually including the staged files in your command string or array by using the argument provided to the function [1][3]. 3. Command Length: Because lint-staged appends these files as arguments, very long lists of files might exceed system limits [5]. lint-staged attempts to mitigate this by automatically splitting the list of files into chunks and running the command multiple times [5]. You can control this behavior with the --max-arg-length option if necessary [5].
Citations:
- 1: https://github.com/lint-staged/lint-staged/blob/main/README.md
- 2: https://github.com/okonet/lint-staged/
- 3: https://github.com/lint-staged/lint-staged
- 4: https://stackoverflow.com/questions/64429228/lint-staged-added-path-to-command
- 5: https://registry.npmjs.org/lint-staged
🌐 Web query:
"project" "file names" TypeScript tsc command line compiler rejects project mode filenames
💡 Result:
The TypeScript compiler (tsc) does not have a hard-coded prohibition against specific project filenames in "project mode" (invoked via --project or --build), but it does have specific behavioral requirements and limitations that can cause errors or unexpected rejection of certain files or paths [1][2][3][4]. Common reasons for issues often mistaken for "rejection" include: 1. Filename Parsing and Path Issues: When using tsc --project, if the path to your project contains spaces, the compiler may incorrectly split the path and attempt to interpret the latter part of the path as a separate source file, triggering error TS5042 (option 'project' cannot be mixed with source files) [1]. 2. Extension and Directory Resolution: - Files ending in.jsonc may be incorrectly treated as directories by certain build commands, leading to errors where tsc attempts to append /tsconfig.json to the path [3]. - If you provide a custom configuration filename that does not end in.json or.jsonc, or if the compiler is configured to look for specific naming conventions, it may fail to locate the project file correctly [3]. 3. File Naming Collisions: The TypeScript Language Server can fail to respect project configuration if source files in the project share the same name but differ only by extension (e.g., index.ts and index.tsx residing in the same directory) [2]. 4. Build Mode vs. Project Mode: Using --project (or -p) is fundamentally different from passing a list of files [1]. Mixing these modes—for example, passing a project file and additional source files on the same command line—is not supported and will be rejected [1]. Similarly, --build (tsc -b) has specific requirements for project configuration [5][4]. To avoid these issues: - Ensure project paths and filenames contain no spaces [1]. - Use consistent, standard extensions (.json or.jsonc) for your configuration files [3]. - Avoid mixing --project or --build modes with explicit source file lists on the command line [1]. - Ensure that source files within the project have unique names if possible, particularly when dealing with.ts and.tsx variants in the same scope [2].
Citations:
- 1: https://stackoverflow.com/questions/35813033/typescript-ts5042build-option-project-cannot-be-mixed-with-source-files-on-a
- 2: The TS Language Server will fail to use the project config when there is a file with both the
.tsand.tsxextensions microsoft/TypeScript#57360 - 3:
tsc --buildand tsconfig project references treat paths ending in.jsoncas directories microsoft/TypeScript#60261 - 4: https://stackoverflow.com/questions/70703085/tsc-build-specify-tsconfig-json
- 5: https://www.typescriptlang.org/docs/handbook/project-references
🌐 Web query:
Cargo CLI -- separator unknown option after separator passed to subcommand
💡 Result:
The double-dash separator (--) in the Cargo CLI is used to distinguish between arguments intended for Cargo itself and arguments intended for the underlying process or tool being invoked (such as a compiled binary, a test harness, or the compiler) [1][2][3]. If you receive an "unknown option" error after the -- separator, it typically means one of the following: 1. Misplaced Arguments: You may be placing arguments intended for Cargo after the -- separator. Arguments placed after -- are passed directly to the sub-process [2][3]. If those arguments contain flags that look like Cargo options (e.g., --verbose), Cargo will not parse them, but the receiving tool might not understand them either, or the CLI parser may still flag them if they are improperly formatted [1][4]. 2. Tool-Specific Argument Parsing: Many tools invoked by Cargo (like test runners or benchmarks) have their own internal argument parsers [4]. If you pass an option that the target tool does not recognize, the error message may originate from that tool rather than Cargo [4]. For example, when running cargo bench or cargo test, arguments after the -- are passed to the test/benchmark harness [4]. Providing an invalid flag to that harness will result in an "unknown option" error [4]. 3. Correct Usage: Ensure that all flags intended for Cargo (such as -p, --release, or --bin) are placed before the -- separator [1][2]. Only pass arguments intended for the final executable or compiler after the -- [2][3]. Example: If you want to run a binary with the flag -r: - Correct: cargo run -- -r [1] - Incorrect: cargo run -r -- (where -r is for the binary) [1] If you believe the option is valid and should be supported by Cargo, ensure it is not a typo or an unstable flag that requires -Z [5][6]. You can verify the valid arguments for a command by running cargo <command> --help [5][7].
Citations:
- 1: Cargo not passing arguments to binary rust-lang/cargo#1855
- 2: https://doc.rust-lang.org/cargo/commands/cargo-run.html
- 3: https://doc.rust-lang.org/stable/cargo/commands/cargo-rustc.html
- 4:
benchdoes not pass dash-prefixed arguments to binary rust-lang/cargo#10228 - 5: Display help menu if user provides an unknow option rust-lang/cargo#7122
- 6: https://dev-doc.rust-lang.org/nightly/cargo/commands/cargo.html
- 7: https://doc.rust-lang.org/cargo/commands/cargo-help.html
Run the compiler and Cargo clippy as filename-agnostic tasks.
With lint-staged string tasks, matched-file paths are appended after the command, so cd backend && tsc --noEmit and cd frontend && tsc --noEmit are executed with source filenames appended alongside project defaults, and cd contracts && cargo clippy -- -D warnings receives those paths after Cargo’s --, where they can be interpreted as rustc flags. Use JS function tasks that ignore filenames, or wrap the checks in scripts that ignore args and run the project-level commands.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@package.json` around lines 43 - 48, Update the lint-staged compiler and Cargo
clippy entries to use filename-agnostic JavaScript functions or
argument-ignoring wrapper scripts, so matched filenames are not appended to
`tsc` or `cargo clippy`; preserve the existing backend, frontend, and contracts
working-directory commands and strict clippy warnings.
|
Hi @Venrable18, This PR could not be merged because it has merge conflicts with the target branch. Please resolve the merge conflicts, push the updated changes, and the PR can be reviewed and merged. Thank you! |
1 similar comment
|
Hi @Venrable18, This PR could not be merged because it has merge conflicts with the target branch. Please resolve the merge conflicts, push the updated changes, and the PR can be reviewed and merged. Thank you! |
2 participants
Closes #640
All acceptance criteria met:
✅ Hook runs < 20 seconds - Completed in 2.264s
✅ Bypass documented (--no-verify) - Added to CONTRIBUTING.md
✅ Setup in CONTRIBUTING.md - Full documentation added
Implementation summary:
Added husky and lint-staged to root package.json
Configured lint-staged to run ESLint, Prettier, and TypeScript checks on backend//*.{ts,tsx} and frontend//.{ts,tsx}
Configured cargo clippy for contracts/**/.rs
Updated CONTRIBUTING.md with hook documentation and bypass instructions
Verified performance: 2.3s for typical changes
Summary by CodeRabbit
Documentation
Chores