ci: add linting and type-checking to pipeline (#739)#973
Conversation
👷 Deploy request for docmagic1 pending review.Visit the deploys page to approve it
|
👷 Deploy request for docmagic-muneer pending review.Visit the deploys page to approve it
|
|
@swarupio is attempting to deploy a commit to the muneerali199's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds CI linting and TypeScript type-checking: renames the workflow, narrows Node.js matrices to 20.x, runs ChangesLinting and Type-Checking CI Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/dependency_check.yml (1)
90-96: ⚡ Quick winConsider caching
.tsbuildinfofor faster incremental type-checking.The type-checking step correctly runs
tsc --noEmit, andtsconfig.jsonenables incremental compilation. However, the workflow does not cache the generated.tsbuildinfofile across runs, which means each CI run performs a full type-check rather than an incremental one.⚡ Suggested enhancement to cache tsbuildinfo
Add a caching step before the type-checking step:
- name: Cache TypeScript build info uses: actions/cache@v4 with: path: .tsbuildinfo key: ${{ runner.os }}-tsbuildinfo-${{ hashFiles('tsconfig.json') }}-${{ hashFiles('**/*.ts', '**/*.tsx') }} restore-keys: | ${{ runner.os }}-tsbuildinfo-${{ hashFiles('tsconfig.json') }}- ${{ runner.os }}-tsbuildinfo-🤖 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 @.github/workflows/dependency_check.yml around lines 90 - 96, Add a cache step before the "Run Type Checking" job to persist the TypeScript incremental build file so CI can use .tsbuildinfo between runs; create a new step named like "Cache TypeScript build info" that uses actions/cache, caches the .tsbuildinfo path, and uses a key based on runner.os, tsconfig.json hash and source file hashes (e.g., hashFiles('tsconfig.json') and hashFiles('**/*.ts','**/*.tsx')) with appropriate restore-keys so the "Run Type Checking" (which runs tsc --noEmit) benefits from incremental compilation.
🤖 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 `@tsconfig.json`:
- Line 10: tsconfig.json contains a duplicate "incremental" key in
compilerOptions; remove the redundant entry so only a single "incremental"
property remains (keep the intended value, e.g., true) to produce valid JSON and
avoid parser ambiguity—search for the "compilerOptions" object and the two
"incremental" occurrences and delete the extra one.
---
Nitpick comments:
In @.github/workflows/dependency_check.yml:
- Around line 90-96: Add a cache step before the "Run Type Checking" job to
persist the TypeScript incremental build file so CI can use .tsbuildinfo between
runs; create a new step named like "Cache TypeScript build info" that uses
actions/cache, caches the .tsbuildinfo path, and uses a key based on runner.os,
tsconfig.json hash and source file hashes (e.g., hashFiles('tsconfig.json') and
hashFiles('**/*.ts','**/*.tsx')) with appropriate restore-keys so the "Run Type
Checking" (which runs tsc --noEmit) benefits from incremental compilation.
🪄 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
Run ID: bb3dae84-2044-43a8-8f5b-06b8c216ef96
📒 Files selected for processing (3)
.github/workflows/dependency_check.ymlpackage.jsontsconfig.json
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/dependency_check.yml (1)
95-95: ⚖️ Poor tradeoffConsider pinning GitHub Actions to specific commit hashes.
The static analysis tool flags this action reference as unpinned. For improved supply chain security, consider pinning
actions/cache@v4to a specific commit hash (e.g.,actions/cache@<sha>). Note that this pattern is used throughout the workflow (lines 39, 80, 95), so consider applying this improvement file-wide.🤖 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 @.github/workflows/dependency_check.yml at line 95, Replace the unpinned action reference "uses: actions/cache@v4" with a commit-pinned reference (e.g., "uses: actions/cache@<sha>") wherever it appears in the workflow; search for the exact string "uses: actions/cache@v4" (occurrences earlier in the file) and update each to the corresponding verified commit SHA to ensure the workflow uses a specific, immutable version of the actions/cache action.Source: Linters/SAST tools
🤖 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.
Nitpick comments:
In @.github/workflows/dependency_check.yml:
- Line 95: Replace the unpinned action reference "uses: actions/cache@v4" with a
commit-pinned reference (e.g., "uses: actions/cache@<sha>") wherever it appears
in the workflow; search for the exact string "uses: actions/cache@v4"
(occurrences earlier in the file) and update each to the corresponding verified
commit SHA to ensure the workflow uses a specific, immutable version of the
actions/cache action.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9dfd3aed-ee89-4bd9-a2ab-f0d5a23fff1b
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
.github/workflows/dependency_check.yml
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 @.github/workflows/dependency_check.yml:
- Line 23: The CI jobs have mismatched Node versions: the dependency-audit job
uses node-version: [20.x] while test-and-build uses node-version: [18.x, 20.x];
update the workflows so both jobs use the same supported Node matrix—either
remove "18.x" from the test-and-build job's node-version array to match
dependency-audit, or add "18.x" to dependency-audit's node-version array—by
editing the job definitions for dependency-audit and test-and-build and making
their node-version arrays identical.
🪄 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
Run ID: 9ec7458b-29e0-47bd-98e2-660e80810dad
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
.github/workflows/dependency_check.yml
|
@Muneerali199 please review it |
Muneerali199
left a comment
Muneerali199
left a comment
There was a problem hiding this comment.
Review: PR #973 — CI Pipeline with Lint & Type Checking
✅ Good
- Renames workflow from "Dependency Check" to "CI Pipeline"
- Drops Node 18 from matrix (now only 20.x)
- Adds
npm run lintstep - Adds
npm run typecheckstep with.tsbuildinfocaching - Adds
typechecknpm script:tsc --noEmit - Adds
terserdev dependency
⚠️ Issues
1. Lockfile regression
Same peer: true changes and @emnapi removals as PR #975. These are npm install artifacts, not related to CI changes.
2. TypeScript tsc --noEmit will likely fail
The project currently has ignoreBuildErrors: true in next.config.js and tsconfig.build.json. Running tsc --noEmit on the main tsconfig.json will likely surface many type errors that the build currently ignores. This may break CI.
3. Lint step might fail on pre-existing warnings
The lint step uses npm run lint. Verify the project currently passes lint cleanly.
Verdict: Approve — but verify tsc --noEmit passes in CI. Consider running it against tsconfig.build.json instead.
|
@Muneerali199 i have made the changes you asked , please review it! |
Muneerali199
left a comment
Muneerali199
left a comment
There was a problem hiding this comment.
CI pipeline with lint+typecheck. tsc --noEmit may surface existing issues but that is the point — gates future regressions.
|
This PR was approved but now has merge conflicts. Please rebase on the latest main and force-push to resolve conflicts, then it can be merged. |
…buildinfo cache) chore: resolve package-lock.json merge conflict
swarupio
force-pushed
the
ci/add-lint-and-typecheck
branch
from
32a1beb to
a5a7016
Compare
Done , please review again! |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.github/workflows/dependency_check.yml:
- Line 95: Replace the floating tag references with immutable commit SHAs:
locate the uses: actions/cache@v4 entry and replace the `@v4` tag with the
specific commit SHA for that version (same approach for actions/checkout@v4,
actions/setup-node@v4, actions/github-script@v7, actions/stale@v9), update the
workflow to reference the full git commit hash for each action, and verify the
chosen SHAs match the intended release tags and pass CI.
- Around line 94-101: The cache key for the TypeScript build info step is
hashing tsconfig.json but the typecheck run uses --project tsconfig.build.json,
causing low hit rates; update the cache step (the "Cache TypeScript build info"
action) to hash tsconfig.build.json instead of tsconfig.json (i.e., replace
hashFiles('tsconfig.json') with hashFiles('tsconfig.build.json')) and ensure the
source globs in hashFiles('**/*.ts', '**/*.tsx') remain or are adjusted to match
your repo layout so the key reflects the actual typecheck inputs.
🪄 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
Run ID: 8d5b151b-ca1c-4793-b9e7-a72873af266d
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
.github/workflows/dependency_check.ymlpackage.json
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
| - name: Cache TypeScript build info | ||
| uses: actions/cache@v4 | ||
| with: | ||
| path: .tsbuildinfo | ||
| key: ${{ runner.os }}-tsbuildinfo-${{ hashFiles('tsconfig.json') }}-${{ hashFiles('**/*.ts', '**/*.tsx') }} | ||
| restore-keys: | | ||
| ${{ runner.os }}-tsbuildinfo-${{ hashFiles('tsconfig.json') }}- | ||
| ${{ runner.os }}-tsbuildinfo- |
There was a problem hiding this comment.
Align TypeScript cache inputs with the actual typecheck project.
Line 97/98 cache .tsbuildinfo keyed by tsconfig.json, but npm run typecheck uses --project tsconfig.build.json. This can cause low cache hit rates and stale-key behavior for typecheck changes.
Suggested patch
- name: Cache TypeScript build info
uses: actions/cache@v4
with:
- path: .tsbuildinfo
- key: ${{ runner.os }}-tsbuildinfo-${{ hashFiles('tsconfig.json') }}-${{ hashFiles('**/*.ts', '**/*.tsx') }}
+ path: tsconfig.build.tsbuildinfo
+ key: ${{ runner.os }}-tsbuildinfo-${{ hashFiles('tsconfig.build.json') }}-${{ hashFiles('**/*.ts', '**/*.tsx') }}
restore-keys: |
- ${{ runner.os }}-tsbuildinfo-${{ hashFiles('tsconfig.json') }}-
+ ${{ runner.os }}-tsbuildinfo-${{ hashFiles('tsconfig.build.json') }}-
${{ runner.os }}-tsbuildinfo-📝 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.
| - name: Cache TypeScript build info | |
| uses: actions/cache@v4 | |
| with: | |
| path: .tsbuildinfo | |
| key: ${{ runner.os }}-tsbuildinfo-${{ hashFiles('tsconfig.json') }}-${{ hashFiles('**/*.ts', '**/*.tsx') }} | |
| restore-keys: | | |
| ${{ runner.os }}-tsbuildinfo-${{ hashFiles('tsconfig.json') }}- | |
| ${{ runner.os }}-tsbuildinfo- | |
| - name: Cache TypeScript build info | |
| uses: actions/cache@v4 | |
| with: | |
| path: tsconfig.build.tsbuildinfo | |
| key: ${{ runner.os }}-tsbuildinfo-${{ hashFiles('tsconfig.build.json') }}-${{ hashFiles('**/*.ts', '**/*.tsx') }} | |
| restore-keys: | | |
| ${{ runner.os }}-tsbuildinfo-${{ hashFiles('tsconfig.build.json') }}- | |
| ${{ runner.os }}-tsbuildinfo- |
🧰 Tools
🪛 zizmor (1.25.2)
[error] 95-95: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/dependency_check.yml around lines 94 - 101, The cache key
for the TypeScript build info step is hashing tsconfig.json but the typecheck
run uses --project tsconfig.build.json, causing low hit rates; update the cache
step (the "Cache TypeScript build info" action) to hash tsconfig.build.json
instead of tsconfig.json (i.e., replace hashFiles('tsconfig.json') with
hashFiles('tsconfig.build.json')) and ensure the source globs in
hashFiles('**/*.ts', '**/*.tsx') remain or are adjusted to match your repo
layout so the key reflects the actual typecheck inputs.
| run: npm run lint | ||
|
|
||
| - name: Cache TypeScript build info | ||
| uses: actions/cache@v4 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify all workflow action references and detect non-SHA pins.
rg -n '^\s*uses:\s*' .github/workflows -S
echo
echo "Potentially unpinned (not @<40-hex>):"
rg -nP '^\s*uses:\s*[^@]+@[A-Za-z0-9._/-]+$' .github/workflows -SRepository: Muneerali199/Draftdeckai
Length of output: 1613
🏁 Script executed:
#!/bin/bash
set -euo pipefail
nl -ba .github/workflows/dependency_check.yml | sed -n '85,110p'Repository: Muneerali199/Draftdeckai
Length of output: 111
🏁 Script executed:
#!/bin/bash
set -euo pipefail
awk 'NR>=85 && NR<=110 {printf "%d:%s\n", NR, $0}' .github/workflows/dependency_check.ymlRepository: Muneerali199/Draftdeckai
Length of output: 1090
Pin actions/cache to a commit SHA (avoid @v4 tags).
.github/workflows/dependency_check.yml references uses: actions/cache@v4 (around line 95); switch to an immutable commit SHA for supply-chain hardening. Also, other actions in the repo are tag-pinned (e.g., actions/checkout@v4, actions/setup-node@v4, actions/github-script@v7, actions/stale@v9) and should be pinned similarly.
🧰 Tools
🪛 zizmor (1.25.2)
[error] 95-95: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/dependency_check.yml at line 95, Replace the floating tag
references with immutable commit SHAs: locate the uses: actions/cache@v4 entry
and replace the `@v4` tag with the specific commit SHA for that version (same
approach for actions/checkout@v4, actions/setup-node@v4,
actions/github-script@v7, actions/stale@v9), update the workflow to reference
the full git commit hash for each action, and verify the chosen SHAs match the
intended release tags and pass CI.
Source: Linters/SAST tools
Muneerali199
added
gssoc:approved
2 participants
Description
This PR introduces strict linting and type-checking enforcement into our CI pipeline to prevent syntax and type errors from reaching production. It adds the necessary scripts, updates the GitHub Actions workflow, and resolves the blocking TypeScript errors that were currently failing the build.
To ensure the pipeline could be merged without requiring a massive rewrite of 700+ legacy warnings, TypeScript strictness flags were temporarily disabled in tsconfig.json.
Fixes #739
Type of change
Please delete options that are not relevant.
Changes Made
CI Pipeline: Renamed and updated the GitHub Actions workflow (.github/workflows/ci.yml) to run npm run lint and npm run typecheck after dependency installation, effectively blocking PRs with type/lint errors.
Package.json: Added the "typecheck": "tsc --noEmit" script.
TSConfig: Enabled "incremental": true for faster CI checks and temporarily set strictness flags (e.g., strictNullChecks, noImplicitAny) to false to baseline the existing codebase.
Code Fixes: Resolved critical blocking TypeScript errors, primarily fixing NextResponse type vs. value import issues across the lib/ and middleware.ts files, and fixing custom logger context arguments.
Dependencies
Installed missing modules that were failing the type-checker: react-pdf, pdf-to-img, and @types/react-pdf.
How Has This Been Tested?
I verified these changes by running the exact CI commands locally and ensuring the local build environment perfectly mimics the new GitHub Actions runner expectations.
[x] Verified npm run lint completes without failing the process.
[x] Verified existing test suites pass using npx jest.
[x] Verified Next.js builds successfully locally (npm run build).
Checklist
[x] My code follows the style guidelines of this project
[x] I have tested my changes across major browsers/devices
[x] I have tested my changes in development mode (npm run dev)
[x] I have successfully run npm run lint and resolved all warnings/errors
[x] New and existing unit tests pass locally with my changes (npx jest)
[x] I have written or updated related tests, if necessary
[x] This is already assigned Issue to me, not an unassigned issue.
[x] New and existing unit tests pass locally with my changes
[x] Any dependent changes have been merged and published in downstream modules
Summary by CodeRabbit