ci: initialize GitHub Actions workflow structure#356
Conversation
📝 WalkthroughWalkthroughThis PR introduces automated CI checks by adding a GitHub Actions workflow and supporting npm scripts. The workflow runs ESLint and TypeScript type-checks for both client and server on push and pull requests, with a gating job that ensures all checks pass before the workflow succeeds. ChangesCI Workflow and Type-Check Integration
Possibly Related Issues
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: 2
🧹 Nitpick comments (4)
.github/workflows/ci.yml (4)
51-78: ⚡ Quick winConsider consistent path filtering across all jobs.
The
lint-clientjob attempts path filtering (lines 26-29), buttypecheck-clientdoesn't. For consistency and efficiency, either all client jobs should use path filtering, or none should. Since the current path filter is broken (see earlier comment), this inconsistency is currently harmless, but should be addressed when implementing proper path filtering.🤖 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/ci.yml around lines 51 - 78, The CI workflow is inconsistent: the lint-client job uses path filtering but typecheck-client does not, so either add the same path filter logic to the typecheck-client job or remove path filtering from lint-client to keep behavior consistent; update the job named "typecheck-client" to include the same path filter condition/if block used by "lint-client" (or remove the path-filtering step from "lint-client") so all client jobs follow the same inclusion logic.
35-36: ⚡ Quick winConsider security hardening: disable credential persistence.
Static analysis flagged that
actions/checkout@v4doesn't setpersist-credentials: false. For defense-in-depth in read-only CI workflows, disabling credential persistence prevents potential credential leakage if the workspace is compromised.🔒 Proposed fix
Apply to all four checkout steps:
- name: Checkout uses: actions/checkout@v4 + with: + persist-credentials: falseAlso applies to: 59-60, 90-91, 123-124
🤖 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/ci.yml around lines 35 - 36, The checkout steps using actions/checkout@v4 should explicitly set persist-credentials: false to avoid leaving GITHUB_TOKEN in the workspace; update each Checkout job step (the occurrences of the checkout action at the shown locations) to include the input persist-credentials: false so credential persistence is disabled for those steps (apply the same change to all four checkout steps referenced).
36-36: ⚖️ Poor tradeoffConsider pinning actions to commit SHAs for supply-chain security.
Static analysis flagged that actions are referenced by tags (
@v4) instead of commit SHAs. Pinning to SHAs prevents tag manipulation attacks and ensures reproducible builds.However, this is a tradeoff:
- Pros: Maximum supply-chain security, immune to tag manipulation
- Cons: Harder to maintain (must manually update SHAs for updates), less readable
Many projects accept the risk of using version tags for maintainability. Choose based on your security posture requirements.
Example SHA pinning
- uses: actions/checkout@v4 + uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1Tools like Dependabot can help automate SHA updates.
Also applies to: 39-39, 60-60, 63-63, 91-91, 94-94, 124-124, 127-127
🤖 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/ci.yml at line 36, The workflow currently references GitHub Actions by mutable tags like actions/checkout@v4 (and similar entries noted at lines referencing actions/setup-node, actions/cache, etc.); replace each tag reference with the corresponding commit SHA to pin the action (e.g., actions/checkout@<full-commit-sha>) by fetching the action repository's commit SHA for the desired tag/release (or use Dependabot to automate updates); update every affected action reference (actions/checkout, actions/setup-node, actions/cache, etc.) in the workflow to use the full commit SHA to ensure reproducible and supply-chain-secure runs.
103-113: ⚡ Quick winUse consistent linting command and expand config detection.
Two concerns:
- Command inconsistency: Client uses
npm run lint(line 49), but server usesnpx eslint .(line 110). For consistency and to allow server-specific ESLint flags in package.json, usenpm run lintonce the script exists.- Incomplete config detection: The check misses
.eslintrc.yml,.eslintrc.yaml, and ESLint config inpackage.json(legacy but still used).♻️ Proposed improvements
- name: Run ESLint # Non-blocking until an eslint.config.js is added to /server. # Once added, remove `continue-on-error` to enforce it as a hard gate. continue-on-error: true run: | - if [ -f eslint.config.js ] || [ -f eslint.config.mjs ] || [ -f eslint.config.cjs ] || [ -f .eslintrc.json ] || [ -f .eslintrc.js ]; then + if [ -f eslint.config.js ] || [ -f eslint.config.mjs ] || [ -f eslint.config.cjs ] || [ -f .eslintrc.* ] || grep -q '"eslintConfig"' package.json 2>/dev/null; then echo "ESLint config found — running lint." - npx eslint . + npm run lint else echo "::warning::No ESLint config in /server. Add eslint.config.js to enforce server-side linting." fiNote: This assumes
npm run lintwill be added toserver/package.jsonwhen ESLint is 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 @.github/workflows/ci.yml around lines 103 - 113, CI job uses npx eslint and misses some config files; switch to the npm script and expand config detection: replace the npx eslint . invocation with npm run lint (so server package.json can control ESLint flags) inside the step that currently runs ESLint, and update the conditional that checks for configs to include .eslintrc.yml, .eslintrc.yaml and package.json having an "eslintConfig" key (e.g., test package.json for '"eslintConfig"') so the job detects legacy configs; keep continue-on-error for now but note it can be removed once the server npm lint script is required.
🤖 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/ci.yml:
- Around line 26-29: The if-condition in the workflow (the multi-line expression
containing github.event_name,
contains(github.event.pull_request.changed_files_url, 'client'), and the
trailing "|| true") is broken because contains() is checking a URL string and
"|| true" makes it always run; remove that entire conditional expression and
implement proper path filtering instead by using workflow-level
paths/paths-ignore under on: (or integrate a path-filter action like
dorny/paths-filter) so the job only runs for changes under the client/** paths
or other intended directories; update the job's if: only if you have a concrete
boolean expression, otherwise delete the if: block so the built-in on: paths
control the triggers.
In `@client/package.json`:
- Line 10: The current npm script named "typecheck" runs "tsc -b", which
requires referenced configs to have "compilerOptions.composite": true; update
the "typecheck" script in package.json to run "tsc --noEmit" instead so it
performs a full typecheck without needing composite project references (leave
"tsc -b" only if you also set composite: true in the referenced configs:
client/tsconfig.json -> tsconfig.app.json and tsconfig.node.json).
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 51-78: The CI workflow is inconsistent: the lint-client job uses
path filtering but typecheck-client does not, so either add the same path filter
logic to the typecheck-client job or remove path filtering from lint-client to
keep behavior consistent; update the job named "typecheck-client" to include the
same path filter condition/if block used by "lint-client" (or remove the
path-filtering step from "lint-client") so all client jobs follow the same
inclusion logic.
- Around line 35-36: The checkout steps using actions/checkout@v4 should
explicitly set persist-credentials: false to avoid leaving GITHUB_TOKEN in the
workspace; update each Checkout job step (the occurrences of the checkout action
at the shown locations) to include the input persist-credentials: false so
credential persistence is disabled for those steps (apply the same change to all
four checkout steps referenced).
- Line 36: The workflow currently references GitHub Actions by mutable tags like
actions/checkout@v4 (and similar entries noted at lines referencing
actions/setup-node, actions/cache, etc.); replace each tag reference with the
corresponding commit SHA to pin the action (e.g.,
actions/checkout@<full-commit-sha>) by fetching the action repository's commit
SHA for the desired tag/release (or use Dependabot to automate updates); update
every affected action reference (actions/checkout, actions/setup-node,
actions/cache, etc.) in the workflow to use the full commit SHA to ensure
reproducible and supply-chain-secure runs.
- Around line 103-113: CI job uses npx eslint and misses some config files;
switch to the npm script and expand config detection: replace the npx eslint .
invocation with npm run lint (so server package.json can control ESLint flags)
inside the step that currently runs ESLint, and update the conditional that
checks for configs to include .eslintrc.yml, .eslintrc.yaml and package.json
having an "eslintConfig" key (e.g., test package.json for '"eslintConfig"') so
the job detects legacy configs; keep continue-on-error for now but note it can
be removed once the server npm lint script is required.
🪄 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: 993766a9-8d9c-4ba4-8355-10197cd9f81e
📒 Files selected for processing (3)
.github/workflows/ci.ymlclient/package.jsonserver/package.json
| if: | | ||
| github.event_name == 'push' || | ||
| contains(github.event.pull_request.changed_files_url, 'client') || | ||
| true |
There was a problem hiding this comment.
Path filtering condition is broken and always evaluates to true.
The condition has two issues:
changed_files_urlis a URL string (e.g., "https://api.github.com/repos/.../pulls/123/files"), not a list of changed files, socontains()checks if the substring "client" appears in the URL, not whether client files changed.|| truemakes the entire condition always true, completely defeating the path filtering mentioned in the comment on line 25.
To implement proper path filtering, either:
- Use
paths/paths-ignorefilters in the workflowon:triggers - Use a dedicated action like
dorny/paths-filterto detect changed paths - Remove the broken condition and always run the job
🔧 Proposed fix: Remove broken condition
lint-client:
name: Lint — Client
runs-on: ubuntu-latest
- # Only run when client-side files actually changed
- if: |
- github.event_name == 'push' ||
- contains(github.event.pull_request.changed_files_url, 'client') ||
- true
defaults:Or use a proper paths filter:
on:
pull_request:
paths:
- 'client/**'
push:
branches:
- main
- dev
paths:
- 'client/**'🤖 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/ci.yml around lines 26 - 29, The if-condition in the
workflow (the multi-line expression containing github.event_name,
contains(github.event.pull_request.changed_files_url, 'client'), and the
trailing "|| true") is broken because contains() is checking a URL string and
"|| true" makes it always run; remove that entire conditional expression and
implement proper path filtering instead by using workflow-level
paths/paths-ignore under on: (or integrate a path-filter action like
dorny/paths-filter) so the job only runs for changes under the client/** paths
or other intended directories; update the job's if: only if you have a concrete
boolean expression, otherwise delete the if: block so the built-in on: paths
control the triggers.
| "dev": "vite", | ||
| "build": "npx puppeteer browsers install chrome && tsc -b && vite build", | ||
| "lint": "eslint .", | ||
| "typecheck": "tsc -b", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check TypeScript configuration and verify typecheck command choice
echo "=== Client TypeScript config files ==="
fd -t f 'tsconfig.*json' client/
echo -e "\n=== Client tsconfig content (checking for composite/references) ==="
fd -t f 'tsconfig.*json' client/ -x cat {}
echo -e "\n=== Server typecheck script for comparison ==="
cat server/package.json | jq -r '.scripts.typecheck // "No typecheck script"'Repository: Sachinchaurasiya360/InternHack
Length of output: 2054
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== client/package.json scripts (typecheck/build) ==="
node -e "const p=require('./client/package.json'); console.log(p.scripts && p.scripts.typecheck, '|', p.scripts && p.scripts.build);"
echo -e "\n=== client/tsconfig.app.json (top-level + compilerOptions/composite/references/noEmit) ==="
cat client/tsconfig.app.json
echo -e "\n=== client/tsconfig.node.json (top-level + compilerOptions/composite/references/noEmit) ==="
cat client/tsconfig.node.jsonRepository: Sachinchaurasiya360/InternHack
Length of output: 2004
🌐 Web query:
TypeScript project references referenced tsconfig must have composite true required error with tsc -b
💡 Result:
If tsconfig.json A has a compilerOptions.references entry that points to tsconfig.json B, then tsconfig.json B must set compilerOptions.composite: true; otherwise building with tsc -b/--build will fail. This requirement is stated in the official TypeScript “Project References” docs: “Referenced projects must have the new composite setting enabled.” [1] Why this matters for tsc -b - tsc --build (tsc -b) is the build orchestrator for a project-reference graph: it finds referenced projects, checks whether they’re up-to-date, and builds out-of-date projects in dependency order. [1] - The docs explain that the composite flag enables the referenced-project mechanism to determine where the referenced project outputs are. [1] What the “composite true required error” means - The actionable meaning is: any tsconfig that is referenced by another project via references must be marked composite. [1] - In real setups, the common failure mode when this is missed is an error of the form “Referenced project … must have setting “composite”: true” (discussed in tooling/issue reports). [2] Fix - In the referenced project (B) tsconfig.json, add: { "compilerOptions": { "composite": true, "declaration": true } } The docs note that enabling composite changes build requirements including that declaration must be turned on. [1] Note on wording: your exact question asks for the precise error text; the official docs state the requirement but don’t quote the literal compiler error message in the page content I found. The requirement itself is clear and authoritative. [1]
Citations:
- 1: https://www.typescriptlang.org/docs/handbook/project-references.html
- 2: Referenced project ... must have setting "composite": true nrwl/nx#27292
Fix typecheck to avoid tsc -b project-references/composite requirements
client/tsconfig.jsonuses project references totsconfig.app.jsonandtsconfig.node.json, but those referenced configs don’t setcompilerOptions.composite: true;tsc -b/tsc --buildrequires referenced projects to be composite.- Align CI typechecking with the server by switching
typechecktotsc --noEmit(and keeptsc -bonly if you also update the referenced tsconfigs for composite).
♻️ Proposed change to use tsc --noEmit
- "typecheck": "tsc -b",
+ "typecheck": "tsc --noEmit",📝 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.
| "typecheck": "tsc -b", | |
| "typecheck": "tsc --noEmit", |
🤖 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 `@client/package.json` at line 10, The current npm script named "typecheck"
runs "tsc -b", which requires referenced configs to have
"compilerOptions.composite": true; update the "typecheck" script in package.json
to run "tsc --noEmit" instead so it performs a full typecheck without needing
composite project references (leave "tsc -b" only if you also set composite:
true in the referenced configs: client/tsconfig.json -> tsconfig.app.json and
tsconfig.node.json).
1 participant
THIS PR IS FOR SOLVING THE ISSUE #331
Overview
Implemented a production-grade GitHub Actions CI workflow to automatically validate pull requests through ESLint and TypeScript type-checking across both client and server applications.
This setup introduces automated quality gates to prevent broken or low-quality code from being merged into the repository.
Changes Made
Added GitHub Actions CI workflow under:
.github/workflows/ci.ymlConfigured automated validation for:
Added independent jobs for:
lint-clientlint-servertypecheck-clienttypecheck-serverConfigured workflow triggers for:
Implemented npm dependency caching for faster workflow execution
Reviewed and aligned existing:
Added implementation documentation:
docs/pr-validation-pipeline.mdValidation Coverage
Client
Server
CI Improvements
Future Extensibility
The workflow structure is designed to support future additions such as:
Expected Impact
Summary by CodeRabbit