feat: add pre-push hook for lint and typecheck validation (#290) - #468
feat: add pre-push hook for lint and typecheck validation (#290)#468NatashaAlker wants to merge 6 commits into
Conversation
Add Husky pre-push git hook that runs lint and TypeScript checks before allowing a push, catching errors locally instead of waiting for CI. - Install husky@9.1.7 with prepare script for auto-setup - Add typecheck script (tsc --noEmit) to all 30 workspace packages - Rename turbo task from type-check to typecheck for consistency - Add typecheck CI job to test.yml mirroring the lint job - Create ticket documentation in docs/tickets/290/ Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…r level These rules were at warning level (Biome default), meaning they didn't cause a non-zero exit code and wouldn't block the pre-push hook. Promote to error so unused variables and parameters are caught before pushing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (16)
✅ Files skipped from review due to trivial changes (11)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAdds a pre-push Husky hook and workspace ChangesPre-push Hook, Workspace Typecheck & CI
Notification service — audit log persistence
Sequence Diagram(s)sequenceDiagram
participant Processor as NotificationProcessor
participant AuditDB as AuditLogDB
participant Email as EmailService
Processor->>AuditDB: createNotificationAuditLog(subscription, payload)
Activate AuditDB
AuditDB-->>Processor: auditLog(created) {id, metadata}
Deactivate AuditDB
Processor->>Email: sendEmail(auditLog, payload)
alt sendEmail succeeds
Email-->>Processor: success
Processor->>AuditDB: updateNotificationStatus(id, "Succeeded")
AuditDB-->>Processor: ok
else sendEmail fails
Email-->>Processor: error
Processor->>AuditDB: updateNotificationStatus(id, "Failed", error.message) rgba(220,20,60,0.5)
AuditDB-->>Processor: ok / error (ignored) rgba(220,20,60,0.5)
Processor-->>Processor: return failed result
end
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bee8b8d9-f4a8-4a90-a527-bc79be23c551
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (38)
.github/workflows/test.yml.husky/pre-pushapps/api/package.jsonapps/crons/package.jsonapps/postgres/package.jsonapps/web/package.jsonbiome.jsondocs/tickets/290/plan.mddocs/tickets/290/tasks.mddocs/tickets/290/ticket.mdlibs/account/package.jsonlibs/admin-pages/package.jsonlibs/api/package.jsonlibs/audit-log/package.jsonlibs/auth/package.jsonlibs/cloud-native-platform/package.jsonlibs/list-search-config/package.jsonlibs/list-types/administrative-court-daily-cause-list/package.jsonlibs/list-types/care-standards-tribunal-weekly-hearing-list/package.jsonlibs/list-types/civil-and-family-daily-cause-list/package.jsonlibs/list-types/common/package.jsonlibs/list-types/court-of-appeal-civil-daily-cause-list/package.jsonlibs/list-types/london-administrative-court-daily-cause-list/package.jsonlibs/list-types/rcj-standard-daily-cause-list/package.jsonlibs/location/package.jsonlibs/notification/package.jsonlibs/notifications/package.jsonlibs/pdf-generation/package.jsonlibs/public-pages/package.jsonlibs/publication/package.jsonlibs/redis/package.jsonlibs/simple-router/package.jsonlibs/subscriptions/package.jsonlibs/system-admin-pages/package.jsonlibs/verified-pages/package.jsonlibs/web-core/package.jsonpackage.jsonturbo.json
| - [x] Add husky devDependency and prepare/type-check scripts to root package.json | ||
| - [x] Run yarn install to install husky | ||
| - [x] Initialize husky and create .husky/pre-push hook | ||
| - [x] Add type-check script to all 30 workspace package.json files | ||
| - [x] Verify yarn type-check passes across all packages |
There was a problem hiding this comment.
Use typecheck consistently in the checklist.
Lines 3, 6, and 7 still reference type-check, but the implemented script name is typecheck. This makes the task log inaccurate.
✏️ Suggested doc fix
-- [x] Add husky devDependency and prepare/type-check scripts to root package.json
+- [x] Add husky devDependency and prepare/typecheck scripts to root package.json
...
-- [x] Add type-check script to all 30 workspace package.json files
-- [x] Verify yarn type-check passes across all packages
+- [x] Add typecheck script to all 30 workspace package.json files
+- [x] Verify yarn typecheck passes across all packages📝 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.
| - [x] Add husky devDependency and prepare/type-check scripts to root package.json | |
| - [x] Run yarn install to install husky | |
| - [x] Initialize husky and create .husky/pre-push hook | |
| - [x] Add type-check script to all 30 workspace package.json files | |
| - [x] Verify yarn type-check passes across all packages | |
| - [x] Add husky devDependency and prepare/typecheck scripts to root package.json | |
| - [x] Run yarn install to install husky | |
| - [x] Initialize husky and create .husky/pre-push hook | |
| - [x] Add typecheck script to all 30 workspace package.json files | |
| - [x] Verify yarn typecheck passes across all packages |
|
|
||
| ### Acceptance criteria | ||
|
|
||
| Make sure there is no lint and typescript error in code before pushing to Github. |
There was a problem hiding this comment.
Tidy acceptance criterion capitalisation.
Line 16 should use TypeScript and GitHub for consistency and correctness.
✏️ Suggested wording
-Make sure there is no lint and typescript error in code before pushing to Github.
+Make sure there are no lint or TypeScript errors in code before pushing to GitHub.📝 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.
| Make sure there is no lint and typescript error in code before pushing to Github. | |
| Make sure there are no lint or TypeScript errors in code before pushing to GitHub. |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~16-~16: The official name of this software platform is spelled with a capital “H”.
Context: ...escript error in code before pushing to Github. ## Comments ### Comment by OgechiOke...
(GITHUB)
| "scripts": { | ||
| "build": "tsc", | ||
| "dev": "tsc --watch" | ||
| "dev": "tsc --watch", | ||
| "typecheck": "tsc --noEmit" | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if libs/audit-log has test files or formatting/linting needs
# Check for test files
fd -e test.ts -e spec.ts . libs/audit-log/
# Check for source files that would need linting
fd -e ts -e tsx . libs/audit-log/src/
# Check if other lib packages have these scripts
rg -A 15 '"scripts"' libs/*/package.json | grep -E '(test|format|lint)'Repository: hmcts/cath-service
Length of output: 6297
🏁 Script executed:
cat -n libs/audit-log/package.jsonRepository: hmcts/cath-service
Length of output: 540
Add missing scripts to package.json.
This library package is missing the required test, test:watch, format, lint, and lint:fix scripts. All peer library modules include these alongside the build and dev scripts. Add the following to the scripts section:
"test": "vitest run",
"test:watch": "vitest watch",
"format": "biome format --write .",
"lint": "biome check .",
"lint:fix": "biome check --write --unsafe ."
🎭 Playwright E2E Test Results0 tests 0 ✅ 0s ⏱️ Results for commit 1dbca55. ♻️ This comment has been updated with latest results. |
The blob-ingestion-notifications E2E test was failing because the payload had empty courtLists, causing notification processing to fail silently. Updated the payload with realistic case data matching the reference data for court_id 9001. Also fixed processUserNotification to update the notification audit log to "Failed" when an error is thrown after log creation, preventing notifications from being stuck in "Pending" state. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
libs/notifications/src/notification/notification-service.ts (1)
155-157: Avoid completely silent fallback-update failures.At Line 156, swallowing the nested failure with an empty catch hides operational signals. Consider logging the secondary error at least once.
Suggested tweak
- if (notification) { - await updateNotificationStatus(notification.notificationId, "Failed", undefined, errorMessage).catch(() => {}); - } + if (notification) { + await updateNotificationStatus(notification.notificationId, "Failed", undefined, errorMessage).catch((updateError) => { + console.warn( + `Failed to update notification status for ${notification.notificationId}`, + updateError + ); + }); + }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a23f872e-8aeb-4b10-9613-c4ab42e87f47
📒 Files selected for processing (3)
e2e-tests/tests/api/blob-ingestion-notifications.spec.tslibs/notifications/src/notification/notification-service.test.tslibs/notifications/src/notification/notification-service.ts
|
- Resolved conflict in blob-ingestion-notifications.spec.ts by accepting master's createValidPayload function (dynamic payload creation) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c5f63534-e566-458c-ab0f-a0b8643cd643
📒 Files selected for processing (26)
.github/workflows/job.test.ymlapps/api/package.jsonapps/crons/package.jsonapps/postgres/package.jsonapps/web/package.jsonlibs/account/package.jsonlibs/admin-pages/package.jsonlibs/api/package.jsonlibs/auth/package.jsonlibs/cloud-native-platform/package.jsonlibs/list-search-config/package.jsonlibs/list-types/administrative-court-daily-cause-list/package.jsonlibs/list-types/care-standards-tribunal-weekly-hearing-list/package.jsonlibs/list-types/civil-and-family-daily-cause-list/package.jsonlibs/list-types/common/package.jsonlibs/list-types/court-of-appeal-civil-daily-cause-list/package.jsonlibs/list-types/london-administrative-court-daily-cause-list/package.jsonlibs/list-types/rcj-standard-daily-cause-list/package.jsonlibs/location/package.jsonlibs/notification/package.jsonlibs/notifications/package.jsonlibs/pdf-generation/package.jsonlibs/public-pages/package.jsonlibs/publication/package.jsonlibs/subscriptions/package.jsonlibs/system-admin-pages/package.json
✅ Files skipped from review due to trivial changes (21)
- libs/account/package.json
- libs/list-search-config/package.json
- libs/list-types/civil-and-family-daily-cause-list/package.json
- libs/public-pages/package.json
- libs/list-types/london-administrative-court-daily-cause-list/package.json
- apps/crons/package.json
- libs/api/package.json
- libs/system-admin-pages/package.json
- libs/list-types/administrative-court-daily-cause-list/package.json
- apps/postgres/package.json
- libs/admin-pages/package.json
- libs/publication/package.json
- libs/list-types/court-of-appeal-civil-daily-cause-list/package.json
- libs/notification/package.json
- libs/list-types/care-standards-tribunal-weekly-hearing-list/package.json
- libs/list-types/common/package.json
- libs/subscriptions/package.json
- libs/location/package.json
- apps/api/package.json
- libs/cloud-native-platform/package.json
- libs/auth/package.json
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/web/package.json
- libs/list-types/rcj-standard-daily-cause-list/package.json
- libs/pdf-generation/package.json
- libs/notifications/package.json
| - name: Setup Node.js | ||
| uses: actions/setup-node@v6 | ||
| with: | ||
| node-version: '24.13.1' | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Node versions declared in .github/workflows/job.test.yml:"
rg -n "node-version:" .github/workflows/job.test.yml
echo
echo "If you maintain a repo-wide Node pin, check it too:"
fd -HI '^\.(nvmrc|node-version)$' . | sed 's#^#- #'Repository: hmcts/cath-service
Length of output: 296
🏁 Script executed:
cat .nvmrcRepository: hmcts/cath-service
Length of output: 70
🏁 Script executed:
head -n 75 .github/workflows/job.test.yml | tail -n 60Repository: hmcts/cath-service
Length of output: 1535
🏁 Script executed:
sed -n '110,130p' .github/workflows/job.test.ymlRepository: hmcts/cath-service
Length of output: 570
Unify Node.js version with the other CI jobs.
Line 71 pins 24.13.1, whilst lint and test use 24.15.0. This diverges from the repo-wide pin in .nvmrc (24.15.0) and can produce inconsistent CI results between jobs.
Suggested fix
- name: Setup Node.js
uses: actions/setup-node@v6
with:
- node-version: '24.13.1'
+ node-version: '24.15.0'📝 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: Setup Node.js | |
| uses: actions/setup-node@v6 | |
| with: | |
| node-version: '24.13.1' | |
| - name: Setup Node.js | |
| uses: actions/setup-node@v6 | |
| with: | |
| node-version: '24.15.0' | |
|
5 participants
Summary
yarn lint && yarn typecheckbefore allowing pushes, catching lint and TypeScript errors locally before CItypecheckscript (tsc --noEmit) to all 30+ workspace packages and atypecheckturbo tasknoUnusedVariablesandnoUnusedFunctionParametersrules from warning to error level so they block pushestype-checkCI job totest.ymlbetween lint and test jobs (existing lint job unchanged)Test plan
debuggerstatement → push blocked with Biome errornumber = "string") → push blocked with TypeScript errorCloses #290
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Tests