chore(deps): upgrade dependencies#2874
Merged
gauthier-th merged 7 commits intodevelopfrom Apr 24, 2026
Merged
Conversation
|
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:
📝 WalkthroughWalkthroughUpdated dependencies; replaced secure-random-password with nanoid for temp passwords; load OpenAPI spec asynchronously via fs/promises + js-yaml; extracted winston DailyRotateFile transports with error handlers; standardized Yup conditional schemas across many settings and user-notification components. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsTelegram.tsx (1)
63-77:⚠️ Potential issue | 🟡 MinorBug:
isfunction signature doesn't match dependency array.The
.when(['types'], ...)has onlytypesin the dependency array, but theisfunction expects two parameters(enabled: boolean, types: number). Since there's noenabledfield being watched, the first parameter will receive thetypesvalue, causing incorrect validation logic.Looking at the Settings-level component (
src/components/Settings/Notifications/NotificationsTelegram.tsx:75-81), the same field uses['enabled', 'types']as dependencies. However, this user-level component doesn't have anenabledfield in the form—it only hastypes.The condition should likely just check
!!types:🐛 Proposed fix
telegramMessageThreadId: Yup.string() - .when(['types'], { - is: (enabled: boolean, types: number) => enabled && !!types, + .when('types', { + is: (types: number) => !!types, then: (schema) => schema .nullable() .required( intl.formatMessage(messages.validationTelegramMessageThreadId) ), otherwise: (schema) => schema.nullable(), })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsTelegram.tsx` around lines 63 - 77, The conditional in telegramMessageThreadId's Yup.when uses ['types'] but its is callback expects two params, causing the first param to be the types value; fix by changing the is callback to match the single dependency — e.g., in the Yup.string().when(['types'], ...) for telegramMessageThreadId, replace the is: (enabled, types) => enabled && !!types with a single-arg check like is: (types) => !!types so the validation triggers correctly when types is present.
🧹 Nitpick comments (1)
server/logger.ts (1)
1-1: Scope theno-consolesuppression to the fallback handlers.Only the two transport error listeners need
console.error; disablingno-consolefor the whole file removes that guardrail for future edits.♻️ Proposed cleanup
-/* eslint-disable no-console */ import path from 'path'; import * as winston from 'winston'; import 'winston-daily-rotate-file'; @@ seerrFileTransport.on('error', (err) => { + // eslint-disable-next-line no-console console.error('Error in seerr file transport:', err); }); @@ machineLogFileTransport.on('error', (err) => { + // eslint-disable-next-line no-console console.error('Error in machine log file transport:', err); });Also applies to: 46-52
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/logger.ts` at line 1, Remove the blanket file-level "/* eslint-disable no-console */" and instead scope the suppression to only the fallback transport error handlers: locate the transport 'error' event listeners that call console.error (the fallback handlers around the transport error listeners) and add an inline comment like "/* eslint-disable-next-line no-console */" immediately before each console.error invocation; leave the rest of the file under normal no-console linting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/entity/User.ts`:
- Line 15: nanoid v5 is ESM-only and breaks CommonJS builds; update the codebase
so CommonJS runtimes won't fail by either (A) downgrading nanoid to a
CJS-compatible release (pin nanoid@3.x in package.json and reinstall so the
existing import at the top of server/entity/User.ts and its use in
generatePassword() continue to work), or (B) switching the static top-level
import in server/entity/User.ts to a dynamic import() inside the
generatePassword() function so the ESM package is loaded at runtime, or (C)
raise the Node engine floor to ^22.12.0 in package.json; pick one approach and
apply it consistently (adjust import/usage in generatePassword() if you choose
dynamic import or update package.json if you choose engine bump).
In `@src/components/Settings/Notifications/NotificationsNtfy/index.tsx`:
- Around line 71-76: In the conditional Yup schema where the topic field uses
.nullable().required(...), replace the wrong validation message reference
messages.validationNtfyUrl with messages.validationNtfyTopic so topic-required
errors show the topic-specific message; update the code in the conditional
returned by the .then handler (the schema .nullable().required(...) call) to use
messages.validationNtfyTopic.
---
Outside diff comments:
In
`@src/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsTelegram.tsx`:
- Around line 63-77: The conditional in telegramMessageThreadId's Yup.when uses
['types'] but its is callback expects two params, causing the first param to be
the types value; fix by changing the is callback to match the single dependency
— e.g., in the Yup.string().when(['types'], ...) for telegramMessageThreadId,
replace the is: (enabled, types) => enabled && !!types with a single-arg check
like is: (types) => !!types so the validation triggers correctly when types is
present.
---
Nitpick comments:
In `@server/logger.ts`:
- Line 1: Remove the blanket file-level "/* eslint-disable no-console */" and
instead scope the suppression to only the fallback transport error handlers:
locate the transport 'error' event listeners that call console.error (the
fallback handlers around the transport error listeners) and add an inline
comment like "/* eslint-disable-next-line no-console */" immediately before each
console.error invocation; leave the rest of the file under normal no-console
linting.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d162ed10-e634-4c85-b2e1-48b32a1c4f55
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (23)
package.jsonserver/entity/User.tsserver/index.tsserver/logger.tssrc/components/Settings/Notifications/NotificationsDiscord.tsxsrc/components/Settings/Notifications/NotificationsEmail.tsxsrc/components/Settings/Notifications/NotificationsGotify/index.tsxsrc/components/Settings/Notifications/NotificationsNtfy/index.tsxsrc/components/Settings/Notifications/NotificationsPushbullet/index.tsxsrc/components/Settings/Notifications/NotificationsPushover/index.tsxsrc/components/Settings/Notifications/NotificationsSlack/index.tsxsrc/components/Settings/Notifications/NotificationsTelegram.tsxsrc/components/Settings/Notifications/NotificationsWebhook/index.tsxsrc/components/Settings/SettingsJellyfin.tsxsrc/components/Settings/SettingsNetwork/index.tsxsrc/components/Settings/SettingsPlex.tsxsrc/components/Settings/SettingsUsers/index.tsxsrc/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsDiscord.tsxsrc/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsPushbullet.tsxsrc/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsPushover.tsxsrc/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsTelegram.tsxsrc/components/UserProfile/UserSettings/UserPasswordChange/index.tsxsrc/utils/polyfillIntl.ts
Collaborator
Shouldn't we update this? Because we are going to merge the nodejs bump pr first before this |
Collaborator
|
Required to use |
gauthier-th
force-pushed
the
upgrade-dependencies
branch
from
a8c7057 to
a9ec11b
Compare
Member
Author
This is not released yet. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 98: package.json still lists "sqlite3" while the PR claims a migration to
"better-sqlite3"; decide and act accordingly: if you intend to fully migrate,
remove the "sqlite3" dependency from package.json and ensure any install/test
scripts or CI use better-sqlite3 (and confirm TypeORM datasource config using
type: 'sqlite' works with better-sqlite3), otherwise update the PR description
to state both drivers are supported; reference the dependency name "sqlite3",
the replacement "better-sqlite3", and the TypeORM datasource config "type:
'sqlite'" when making the change.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e2e0a302-daf4-41e1-abfc-40c274897da9
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (23)
package.jsonserver/entity/User.tsserver/index.tsserver/logger.tssrc/components/Settings/Notifications/NotificationsDiscord.tsxsrc/components/Settings/Notifications/NotificationsEmail.tsxsrc/components/Settings/Notifications/NotificationsGotify/index.tsxsrc/components/Settings/Notifications/NotificationsNtfy/index.tsxsrc/components/Settings/Notifications/NotificationsPushbullet/index.tsxsrc/components/Settings/Notifications/NotificationsPushover/index.tsxsrc/components/Settings/Notifications/NotificationsSlack/index.tsxsrc/components/Settings/Notifications/NotificationsTelegram.tsxsrc/components/Settings/Notifications/NotificationsWebhook/index.tsxsrc/components/Settings/SettingsJellyfin.tsxsrc/components/Settings/SettingsNetwork/index.tsxsrc/components/Settings/SettingsPlex.tsxsrc/components/Settings/SettingsUsers/index.tsxsrc/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsDiscord.tsxsrc/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsPushbullet.tsxsrc/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsPushover.tsxsrc/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsTelegram.tsxsrc/components/UserProfile/UserSettings/UserPasswordChange/index.tsxsrc/utils/polyfillIntl.ts
✅ Files skipped from review due to trivial changes (11)
- src/components/Settings/Notifications/NotificationsPushover/index.tsx
- server/index.ts
- src/components/Settings/SettingsJellyfin.tsx
- src/components/Settings/SettingsUsers/index.tsx
- src/components/Settings/Notifications/NotificationsGotify/index.tsx
- src/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsTelegram.tsx
- src/components/Settings/SettingsNetwork/index.tsx
- src/components/Settings/Notifications/NotificationsEmail.tsx
- src/utils/polyfillIntl.ts
- src/components/Settings/Notifications/NotificationsNtfy/index.tsx
- src/components/Settings/Notifications/NotificationsWebhook/index.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- src/components/UserProfile/UserSettings/UserPasswordChange/index.tsx
- src/components/Settings/Notifications/NotificationsPushbullet/index.tsx
- server/logger.ts
- src/components/Settings/Notifications/NotificationsDiscord.tsx
- src/components/Settings/Notifications/NotificationsSlack/index.tsx
seerr
 |
||||||||||||||||||||||||||||
| Project |
seerr
|
| Branch Review |
develop
|
| Run status |
|
| Run duration | 02m 32s |
| Commit |
|
| Committer | Gauthier |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
 |
0
|
 |
1
|
 |
0
|
 |
0
|
 |
32
|
| View all changes introduced in this branch ↗︎ | |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR upgrades a large set of runtime/dev dependencies and performs the necessary refactors to keep server startup, logging, i18n polyfills, and form validation compatible with the new major versions.
Changes:
- Updated many dependencies (including
@formatjs/*,yup,winston-daily-rotate-file,undici) and added config to disable Scarf telemetry. - Replaced unmaintained packages (
yamljs→js-yaml,secure-random-password→nanoid) and updated server code accordingly. - Refactored multiple Yup schemas to the newer
.when(..., { then: (schema) => ... })style.
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/polyfillIntl.ts | Updates @formatjs/* polyfill imports for new package entrypoints. |
| src/components/UserProfile/UserSettings/UserPasswordChange/index.tsx | Adjusts password confirmation validation for new Yup behavior. |
| src/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsTelegram.tsx | Refactors Yup .when schema for Telegram user notification settings. |
| src/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsPushover.tsx | Refactors Yup .when schema for Pushover user settings. |
| src/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsPushbullet.tsx | Refactors Yup .when schema for Pushbullet user settings. |
| src/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsDiscord.tsx | Refactors Yup .when schema for Discord user settings. |
| src/components/Settings/SettingsUsers/index.tsx | Tightens typing in Yup custom test for auth method selection. |
| src/components/Settings/SettingsPlex.tsx | Refactors Yup .when schema for Tautulli-related fields. |
| src/components/Settings/SettingsNetwork/index.tsx | Refactors Yup .when schema and adds nullable branches for disabled fields. |
| src/components/Settings/SettingsJellyfin.tsx | Refactors Yup .when schema for port field. |
| src/components/Settings/Notifications/NotificationsWebhook/index.tsx | Refactors Yup .when schema for webhook notification settings. |
| src/components/Settings/Notifications/NotificationsTelegram.tsx | Refactors Yup .when schema for Telegram agent settings. |
| src/components/Settings/Notifications/NotificationsSlack/index.tsx | Refactors Yup .when schema for Slack agent settings. |
| src/components/Settings/Notifications/NotificationsPushover/index.tsx | Refactors Yup .when schema for Pushover agent settings. |
| src/components/Settings/Notifications/NotificationsPushbullet/index.tsx | Refactors Yup .when schema for Pushbullet agent settings. |
| src/components/Settings/Notifications/NotificationsNtfy/index.tsx | Refactors Yup .when schema for ntfy agent settings. |
| src/components/Settings/Notifications/NotificationsGotify/index.tsx | Refactors Yup .when schema for Gotify agent settings. |
| src/components/Settings/Notifications/NotificationsEmail.tsx | Refactors Yup .when schema for email agent settings. |
| src/components/Settings/Notifications/NotificationsDiscord.tsx | Refactors Yup .when schema for Discord agent settings. |
| server/logger.ts | Adds transport-level error handling for rotating file logs. |
| server/index.ts | Replaces yamljs with js-yaml and reads the API spec via fs/promises. |
| server/entity/User.ts | Replaces password generator with nanoid for generated-password emails. |
| package.json | Applies dependency upgrades/removals and disables Scarf telemetry. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
gauthier-th
force-pushed
the
upgrade-dependencies
branch
from
a9ec11b to
574439d
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsTelegram.tsx (1)
63-77:⚠️ Potential issue | 🟠 MajorBug:
.when()parameter count mismatch.Line 64 uses
.when(['types'], {...})which watches only the'types'field, but theisfunction expects two parameters:is: (enabled: boolean, types: number) => enabled && !!types,Yup passes values in the order of watched fields—since only one field is watched, only one value is passed. This causes:
enabledto receive thetypesvaluetypesto receiveundefined- The condition
enabled && !!typesto always be falsy- The
thenbranch (requiring the field) to never executeAdditionally, there is no
enabledfield ininitialValues(lines 86–91).The correct pattern is shown at line 51:
.when('types', { is: (types: number) => !!types, // ... })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsTelegram.tsx` around lines 63 - 77, The conditional on telegramMessageThreadId uses .when(['types'], ...) but the is callback expects two params (enabled, types) and there is no enabled in initialValues, so update the .when on telegramMessageThreadId to watch only 'types' and change the is signature to a single param (types) using !!types for the condition, or alternatively watch both fields (e.g., ['enabled','types']) if you intend to rely on enabled; ensure the then branch remains the required/nullable schema and remove any reference to an undefined enabled variable so the requirement triggers correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@src/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsTelegram.tsx`:
- Around line 63-77: The conditional on telegramMessageThreadId uses
.when(['types'], ...) but the is callback expects two params (enabled, types)
and there is no enabled in initialValues, so update the .when on
telegramMessageThreadId to watch only 'types' and change the is signature to a
single param (types) using !!types for the condition, or alternatively watch
both fields (e.g., ['enabled','types']) if you intend to rely on enabled; ensure
the then branch remains the required/nullable schema and remove any reference to
an undefined enabled variable so the requirement triggers correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 47bd29c0-cd7a-4bca-8d71-13eb9bdc5d4c
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (23)
package.jsonserver/entity/User.tsserver/index.tsserver/logger.tssrc/components/Settings/Notifications/NotificationsDiscord.tsxsrc/components/Settings/Notifications/NotificationsEmail.tsxsrc/components/Settings/Notifications/NotificationsGotify/index.tsxsrc/components/Settings/Notifications/NotificationsNtfy/index.tsxsrc/components/Settings/Notifications/NotificationsPushbullet/index.tsxsrc/components/Settings/Notifications/NotificationsPushover/index.tsxsrc/components/Settings/Notifications/NotificationsSlack/index.tsxsrc/components/Settings/Notifications/NotificationsTelegram.tsxsrc/components/Settings/Notifications/NotificationsWebhook/index.tsxsrc/components/Settings/SettingsJellyfin.tsxsrc/components/Settings/SettingsNetwork/index.tsxsrc/components/Settings/SettingsPlex.tsxsrc/components/Settings/SettingsUsers/index.tsxsrc/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsDiscord.tsxsrc/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsPushbullet.tsxsrc/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsPushover.tsxsrc/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsTelegram.tsxsrc/components/UserProfile/UserSettings/UserPasswordChange/index.tsxsrc/utils/polyfillIntl.ts
✅ Files skipped from review due to trivial changes (11)
- src/components/Settings/SettingsUsers/index.tsx
- src/components/Settings/Notifications/NotificationsPushbullet/index.tsx
- server/entity/User.ts
- src/components/Settings/Notifications/NotificationsGotify/index.tsx
- src/components/Settings/Notifications/NotificationsTelegram.tsx
- src/utils/polyfillIntl.ts
- src/components/Settings/Notifications/NotificationsWebhook/index.tsx
- server/index.ts
- src/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsPushbullet.tsx
- src/components/Settings/SettingsJellyfin.tsx
- src/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsPushover.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- src/components/UserProfile/UserSettings/UserPasswordChange/index.tsx
- src/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsDiscord.tsx
- server/logger.ts
- src/components/Settings/SettingsPlex.tsx
- src/components/Settings/Notifications/NotificationsPushover/index.tsx
- src/components/Settings/Notifications/NotificationsNtfy/index.tsx
Collaborator
Yes. Added as a comment here for documenting |
gauthier-th
force-pushed
the
upgrade-dependencies
branch
from
574439d to
a46bb17
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 104: Update the proxy agent setup in server/utils/customProxyAgent.ts to
handle undici 8.x's default HTTP/2 behavior: test connectivity with
undici@^8.1.0 and, if the proxy fails when HTTP/2 is used, pass allowH2: false
into the ProxyAgent constructor options (the ProxyAgent instantiation in
customProxyAgent.ts) to force HTTP/1.1; verify the code paths that call
setGlobalDispatcher or ProxyAgent still work with the updated options.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 82f81f05-78f8-4e39-8934-69cdccd9ff76
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (23)
package.jsonserver/entity/User.tsserver/index.tsserver/logger.tssrc/components/Settings/Notifications/NotificationsDiscord.tsxsrc/components/Settings/Notifications/NotificationsEmail.tsxsrc/components/Settings/Notifications/NotificationsGotify/index.tsxsrc/components/Settings/Notifications/NotificationsNtfy/index.tsxsrc/components/Settings/Notifications/NotificationsPushbullet/index.tsxsrc/components/Settings/Notifications/NotificationsPushover/index.tsxsrc/components/Settings/Notifications/NotificationsSlack/index.tsxsrc/components/Settings/Notifications/NotificationsTelegram.tsxsrc/components/Settings/Notifications/NotificationsWebhook/index.tsxsrc/components/Settings/SettingsJellyfin.tsxsrc/components/Settings/SettingsNetwork/index.tsxsrc/components/Settings/SettingsPlex.tsxsrc/components/Settings/SettingsUsers/index.tsxsrc/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsDiscord.tsxsrc/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsPushbullet.tsxsrc/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsPushover.tsxsrc/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsTelegram.tsxsrc/components/UserProfile/UserSettings/UserPasswordChange/index.tsxsrc/utils/polyfillIntl.ts
✅ Files skipped from review due to trivial changes (7)
- src/components/Settings/Notifications/NotificationsPushbullet/index.tsx
- server/entity/User.ts
- src/components/Settings/SettingsJellyfin.tsx
- src/components/Settings/SettingsUsers/index.tsx
- src/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsPushover.tsx
- src/components/Settings/Notifications/NotificationsPushover/index.tsx
- src/utils/polyfillIntl.ts
🚧 Files skipped from review as they are similar to previous changes (11)
- src/components/UserProfile/UserSettings/UserPasswordChange/index.tsx
- src/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsPushbullet.tsx
- src/components/Settings/Notifications/NotificationsSlack/index.tsx
- src/components/Settings/Notifications/NotificationsDiscord.tsx
- src/components/Settings/Notifications/NotificationsWebhook/index.tsx
- src/components/Settings/Notifications/NotificationsGotify/index.tsx
- server/index.ts
- src/components/Settings/Notifications/NotificationsEmail.tsx
- src/components/Settings/SettingsPlex.tsx
- server/logger.ts
- src/components/Settings/Notifications/NotificationsTelegram.tsx
Member
Author
|
Just tested this a bit more in-depth. No issue so far. |
Collaborator
Just tested HTTP(S) proxy: |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
package.json (1)
104-104:⚠️ Potential issue | 🟠 MajorHTTP(S) proxy connectivity confirmed broken with undici 8.x.
The PR comments report a test failure when connecting to proxy:
"Failed to connect to the proxy: Parse Error: Data after
Connection: close"This confirms the concern from the previous review about undici 8.0's HTTP/2 default behavior. Add
allowH2: falseto the ProxyAgent constructor inserver/utils/customProxyAgent.tsto force HTTP/1.1, or consider deferring this upgrade until the root cause is resolved.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 104, The undici 8.x change causes proxy connections to attempt HTTP/2 and fail; update the ProxyAgent construction in server/utils/customProxyAgent.ts (the ProxyAgent(...) call) to pass allowH2: false in its options so the agent forces HTTP/1.1 (e.g., include allowH2: false in the options object passed to new ProxyAgent or ProxyAgent.create); alternatively revert/remove the undici upgrade until a fuller fix is available.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 73: The project depends on nanoid 5.x which is ESM-only and causes
ERR_REQUIRE_ESM when imported from server/entity/User.ts (line ~15) in a
CommonJS build; fix by making the dependency CommonJS-compatible — either
downgrade the package.json entry "nanoid" to a 4.x release (e.g., "^4.0.0") so
existing CommonJS/transpiled imports in server/entity/User.ts continue to work,
or switch the build to ESM (update tsconfig.json "module" to an ESM target and
ensure Node runtime >=18.19) so nanoid@5 can be used; pick one approach and
update package.json (or tsconfig.json and runtime) and adjust any imports in
server/entity/User.ts accordingly.
---
Duplicate comments:
In `@package.json`:
- Line 104: The undici 8.x change causes proxy connections to attempt HTTP/2 and
fail; update the ProxyAgent construction in server/utils/customProxyAgent.ts
(the ProxyAgent(...) call) to pass allowH2: false in its options so the agent
forces HTTP/1.1 (e.g., include allowH2: false in the options object passed to
new ProxyAgent or ProxyAgent.create); alternatively revert/remove the undici
upgrade until a fuller fix is available.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 542f353c-5d7c-4a59-b94f-c59cb7630fb5
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
package.jsonsrc/components/Slider/index.tsx
✅ Files skipped from review due to trivial changes (1)
- src/components/Slider/index.tsx
|
This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged. |
gauthier-th
force-pushed
the
upgrade-dependencies
branch
from
0cc7dd5 to
5c3150a
Compare
github-actions
Bot
removed
the
merge conflict
fallenbagel
requested changes
Apr 18, 2026
M0NsTeRRR
previously approved these changes
Apr 18, 2026
Member
M0NsTeRRR
left a comment
M0NsTeRRR
left a comment
There was a problem hiding this comment.
LGTM, I tested with a little setup without every options like HTTP proxy.
Member
Author
Tested HTTP(s) proxy and everything is working properly. |
M0NsTeRRR
previously approved these changes
Apr 21, 2026
fallenbagel
requested changes
Apr 23, 2026
M0NsTeRRR
previously approved these changes
Apr 24, 2026
gauthier-th
force-pushed
the
upgrade-dependencies
branch
from
2ebd5d4 to
a846ccf
Compare
M0NsTeRRR
approved these changes
Apr 24, 2026
fallenbagel
approved these changes
Apr 24, 2026
Collaborator
|
LGTM. Haven't tested the latest commit but did test the commit before that. Should be fine imo |
7 tasks
7 tasks
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Notes
, we should check the
setGlobalDispatcherbehavior, meaning that we should ensure that our proxy setting still works.pnpm dedupeconnect-typeorm.Changes related to package updates
yamljspackage in favor ofjs-yamlfor parsing YAML files.secure-random-passwordin favor ofnanoidfor generating random strings.@formatjs/intl-utilspackage.@formatjs/intl-...imports to be compatible with the latest version of@formatjs/intl.v5ofswagger-ui-dist.cookie-parsersince it's not needed anymore inexpress-session@^1.5.0.Packages not updated
typescript,express,next,react& co) was not updated in this PR.http-proxy-agent:v8is ESM-onlyhttps-proxy-agent:v8is ESM-onlyhusky:v9is ESM-onlyreact-intlwas updated tov7and not higher because next version require React 19.tailwindcss,tailwind-mergeand@headlessui/react) was not updated as well.sqlite3only updated to v5 because v6 is not supported by TypeORM.react-toast-notificationsis not outdated but is not maintained anymore.react-hot-toastshould be used instead.How Has This Been Tested?
N/A
Checklist:
pnpm buildpnpm i18n:extractSummary by CodeRabbit
Chores
Refactor
Bug Fixes