Conversation
WalkthroughThis PR migrates the project's package manager from npm to pnpm across all configuration, workflow, and documentation files. It updates CI/CD pipelines, Docker configurations, development scripts, and adds pnpm-specific workspace configuration while upgrading Node.js to version 24 in workflows. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2817 +/- ##
=============================================
- Coverage 96.75% 96.22% -0.53%
Complexity 1825 1825
=============================================
Files 440 263 -177
Lines 12479 6203 -6276
Branches 2073 0 -2073
=============================================
- Hits 12074 5969 -6105
+ Misses 405 234 -171 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)
288-299:⚠️ Potential issue | 🔴 Critical
cypress-io/github-actionwill fail due to pnpm enforcement inpreinstallscript.The
cypress-io/github-action@v6defaults to runningnpm cifor dependency installation. However,package.jsonnow has apreinstallscript (npx only-allow pnpm) that will reject any non-pnpm package manager. This will cause the frontend-tests, visual-tests, and system-tests jobs to fail.You need to either:
- Configure the action to use pnpm by adding
install-command: pnpm install(and ensure pnpm is set up in those jobs), or- Set
install: falseand handle installation separately with pnpm.Option 1: Configure cypress action to use pnpm (for frontend-tests job)
Add pnpm/Node setup steps before the Cypress action and configure it:
+ - name: Install pnpm + uses: pnpm/action-setup@v4 + with: + version: 10 + - name: Use Node.js 24 + uses: actions/setup-node@v4 + with: + node-version: 24 + cache: "pnpm" - name: Cypress run frontend tests uses: cypress-io/github-action@v6 with: + install-command: pnpm install wait-on: "http://localhost:9080"Apply the same pattern to the visual-tests and system-tests jobs.
🤖 Fix all issues with AI agents
In @.github/dependabot.yml:
- Around line 8-9: The dependabot configuration uses package-ecosystem: "pnpm"
which is unsupported; update the package-ecosystem key in .github/dependabot.yml
from "pnpm" to "npm" so Dependabot detects pnpm via pnpm-lock.yaml automatically
and continues to manage pnpm dependencies and updates.
In `@docker/app/playback-player/build.sh`:
- Around line 34-35: The build script currently uses pnpm (lines containing
"pnpm install" and "pnpm run build") against the upstream bbb-playback project
which provides an npm lockfile; change the script to respect the project's
npm-based dependency management by either running "npm install" and "npm run
build" instead of pnpm commands, or if you must use pnpm, run "pnpm import"
first to convert the package-lock.json into a pnpm lockfile and then run "pnpm
install" before the build; update the commands in build.sh accordingly so the
repository's package manager and lockfile are handled consistently.
In `@docker/sail/bin/sail`:
- Around line 63-66: Help text advertises "sail pnpx" but the dispatch only
handles "npx" so running "sail pnpx" falls through; add a "pnpx" case in the
command dispatch (the same section that contains the "npx" handler around the
350-360 area) that mirrors the existing npx handling (or simply treat "pnpx" as
an alias to the npx block) so the advertised command is actually handled, and
ensure the echo help lines remain accurate if you instead choose to remove the
pnpx help entry.
In `@pnpm-workspace.yaml`:
- Around line 1-11: The pnpm config contains unsupported v10+ keys and an
inconsistent indent: remove or upgrade—either delete the keys
ignoredBuiltDependencies and onlyBuiltDependencies (they're invalid in pnpm v9)
or upgrade the project to pnpm v10+ so those keys are honored; also fix the
packages list indentation so the 'docs' entry uses two-space indentation like
the rest (ensure packages: followed by " - 'docs'" uses consistent spacing);
finally confirm sideEffectsCache: false is intentional. Use the exact key names
ignoredBuiltDependencies, onlyBuiltDependencies, sideEffectsCache, and packages
to locate the entries.
🧹 Nitpick comments (2)
.github/workflows/deploy-docs.yml (1)
26-36: Consider using--frozen-lockfilefor CI installs.In CI pipelines,
pnpm install --frozen-lockfileensures the lockfile is not modified and the build is fully reproducible. Without it, a mismatch betweenpackage.jsonand the lockfile could silently update dependencies.Suggested fix
- name: Install dependencies - run: pnpm install + run: pnpm install --frozen-lockfile.github/workflows/ci.yml (1)
168-182: Use--frozen-lockfilefor reproducible CI installs.Same as the deploy-docs workflow —
pnpm install --frozen-lockfileshould be used in CI to prevent silent lockfile changes.Suggested fix
- name: Install dependencies - run: pnpm install + run: pnpm install --frozen-lockfile
| # Maintain dependencies for pnpm | ||
| - package-ecosystem: "pnpm" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
GitHub Dependabot pnpm support package ecosystem
💡 Result:
Dependabot treats pnpm as part of the npm package ecosystem. So in .github/dependabot.yml you should use:
updates:
- package-ecosystem: "npm"
directory: "/"
schedule:
interval: "weekly"Dependabot will detect pnpm usage from files like pnpm-lock.yaml (and workspace setup) while still using the npm ecosystem key. pnpm support: version updates work for pnpm v7–v10, while security updates are supported for pnpm v7–v8 only. [1][2]
Also, Dependabot added GA support for pnpm workspace catalogs (monorepo catalogs in pnpm-workspace.yaml) on February 4, 2025. [3]
Sources: [1] [2] [3]
Change package-ecosystem back to "npm" — pnpm is not a supported ecosystem value in Dependabot. Instead, use "npm" and Dependabot will automatically detect pnpm from pnpm-lock.yaml. Note: pnpm version updates are supported for v7–v10, while security updates are supported for v7–v8 only.
🤖 Prompt for AI Agents
In @.github/dependabot.yml around lines 8 - 9, The dependabot configuration uses
package-ecosystem: "pnpm" which is unsupported; update the package-ecosystem key
in .github/dependabot.yml from "pnpm" to "npm" so Dependabot detects pnpm via
pnpm-lock.yaml automatically and continues to manage pnpm dependencies and
updates.
| pnpm install | ||
| pnpm run build |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "Dockerfile" docker/ --exec cat -n {}Repository: THM-Health/PILOS
Length of output: 4584
🏁 Script executed:
cat -n docker/app/playback-player/build.shRepository: THM-Health/PILOS
Length of output: 1565
🏁 Script executed:
cat -n docker/app/pilos-cliRepository: THM-Health/PILOS
Length of output: 2504
🌐 Web query:
bbb-playback GitHub bigbluebutton package.json pnpm support
💡 Result:
bigbluebutton/bbb-playback is set up as an npm project (the README instructs npm install, and the repo ships a package-lock.json). [1] [2]
Because it does not ship a pnpm-lock.yaml and documents npm, pnpm isn’t officially supported/configured out-of-the-box—you can likely make it work, but you’d be doing it as an unsupported workflow (you’d need to generate/maintain pnpm’s lockfile yourself and ensure CI/deploy uses pnpm consistently). [1] [2] (pnpm itself is a separate package manager with its own lockfile, pnpm-lock.yaml.) [3]
Consider respecting the bbb-playback project's npm-based dependency management.
The external bbb-playback project is configured for npm (ships package-lock.json and documents npm install in its README), not pnpm. Running pnpm install on it will work but represents an unsupported workflow—pnpm will use its own dependency resolution instead of respecting the upstream lockfile, and you'll need to manage pnpm's pnpm-lock.yaml separately. Either run pnpm import to properly migrate the lockfile, or use npm to keep consistency with the upstream project's configured package manager.
🤖 Prompt for AI Agents
In `@docker/app/playback-player/build.sh` around lines 34 - 35, The build script
currently uses pnpm (lines containing "pnpm install" and "pnpm run build")
against the upstream bbb-playback project which provides an npm lockfile; change
the script to respect the project's npm-based dependency management by either
running "npm install" and "npm run build" instead of pnpm commands, or if you
must use pnpm, run "pnpm import" first to convert the package-lock.json into a
pnpm lockfile and then run "pnpm install" before the build; update the commands
in build.sh accordingly so the repository's package manager and lockfile are
handled consistently.
| echo "${YELLOW}PNPM Commands:${NC}" | ||
| echo " ${GREEN}sail pnpm ...${NC} Run a pnpm command" | ||
| echo " ${GREEN}sail pnpx${NC} Run a pnpx command" | ||
| echo " ${GREEN}sail pnpm run prod${NC}" |
There was a problem hiding this comment.
sail pnpx is advertised but has no command handler — it will silently fall through to docker-compose.
Line 65 documents sail pnpx, but the dispatch section (lines 350-360) only handles "npx", not "pnpx". Running sail pnpx will pass the arguments unhandled to docker-compose and fail.
Either add a pnpx dispatch block or update the help text to match what's actually handled.
Option A: Add a pnpx handler (mirrors the npx block)
+# Proxy PNPX commands to the "pnpx" binary on the application container...
+elif [ "$1" == "pnpx" ]; then
+ shift 1
+
+ if [ "$EXEC" == "yes" ]; then
+ ARGS+=(exec -u "$APP_USER")
+ [ ! -t 0 ] && ARGS+=(-T)
+ ARGS+=("$APP_SERVICE" pnpx)
+ else
+ sail_is_not_running
+ fi
+
# Proxy NPX commands to the "npx" binary on the application container...
elif [ "$1" == "npx" ]; thenOption B: Update help text to reflect existing handlers
echo "${YELLOW}PNPM Commands:${NC}"
echo " ${GREEN}sail pnpm ...${NC} Run a pnpm command"
- echo " ${GREEN}sail pnpx${NC} Run a pnpx command"
+ echo " ${GREEN}sail npx${NC} Run a npx command"
echo " ${GREEN}sail pnpm run prod${NC}"📝 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.
| echo "${YELLOW}PNPM Commands:${NC}" | |
| echo " ${GREEN}sail pnpm ...${NC} Run a pnpm command" | |
| echo " ${GREEN}sail pnpx${NC} Run a pnpx command" | |
| echo " ${GREEN}sail pnpm run prod${NC}" | |
| echo "${YELLOW}PNPM Commands:${NC}" | |
| echo " ${GREEN}sail pnpm ...${NC} Run a pnpm command" | |
| echo " ${GREEN}sail npx${NC} Run a npx command" | |
| echo " ${GREEN}sail pnpm run prod${NC}" |
🤖 Prompt for AI Agents
In `@docker/sail/bin/sail` around lines 63 - 66, Help text advertises "sail pnpx"
but the dispatch only handles "npx" so running "sail pnpx" falls through; add a
"pnpx" case in the command dispatch (the same section that contains the "npx"
handler around the 350-360 area) that mirrors the existing npx handling (or
simply treat "pnpx" as an alias to the npx block) so the advertised command is
actually handled, and ensure the echo help lines remain accurate if you instead
choose to remove the pnpx help entry.
| ignoredBuiltDependencies: | ||
| - esbuild | ||
| - vue-demi | ||
|
|
||
| onlyBuiltDependencies: | ||
| - cypress | ||
|
|
||
| sideEffectsCache: false | ||
|
|
||
| packages: | ||
| - 'docs' No newline at end of file |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find pnpm-workspace.yaml and check its content
find . -name "pnpm-workspace.yaml" -type f | head -5Repository: THM-Health/PILOS
Length of output: 81
🏁 Script executed:
# Check for package.json and pnpm version specification
find . -name "package.json" -type f | grep -v node_modules | head -3Repository: THM-Health/PILOS
Length of output: 94
🏁 Script executed:
# Also check for .npmrc or pnpm-lock.yaml for version hints
find . \( -name ".npmrc" -o -name "pnpm-lock.yaml" \) -type f | head -5Repository: THM-Health/PILOS
Length of output: 85
🏁 Script executed:
# Read pnpm-workspace.yaml with line numbers to check indentation
cat -n ./pnpm-workspace.yamlRepository: THM-Health/PILOS
Length of output: 269
🏁 Script executed:
# Check pnpm version in package.json
cat ./package.json | grep -A 2 -B 2 "packageManager"Repository: THM-Health/PILOS
Length of output: 42
🏁 Script executed:
# Also check for pnpm version in pnpm-lock.yaml header
head -20 ./pnpm-lock.yamlRepository: THM-Health/PILOS
Length of output: 477
🌐 Web query:
pnpm workspace ignoredBuiltDependencies onlyBuiltDependencies sideEffectsCache v9
💡 Result:
In pnpm v9, these specific settings behave like this:
onlyBuiltDependencies: not available in v9 (introduced in v10.19.0). In v10+, it’s an allowlist of dependencies permitted to runpreinstall/install/postinstall. [1]ignoredBuiltDependencies: not available in v9 (introduced in v10.1.0). In v10+, it’s a denylist that also suppresses the “ignored build scripts” warning/prompt. [1], [2]sideEffectsCache: available; defaults totrueand caches the results of install hooks (pre/postinstall) in the store for faster future installs. [1]
What to use in pnpm v9 instead
- To control lifecycle scripts in v9 you generally use:
ignoreScripts(disable all scripts), orignoreDepScripts/.npmrc: ignore-dep-scripts(don’t run dependency scripts). [1], [3]
- If you specifically want the v10-style “allow only these deps to build” workflow (
onlyBuiltDependencies/ignoredBuiltDependencies), you’ll need to upgrade to pnpm v10+. [1], [2]
Sources: pnpm settings reference for sideEffectsCache, and the “Added in” versions for onlyBuiltDependencies / ignoredBuiltDependencies [1]; pnpm package.json field docs for pnpm.ignoredBuiltDependencies [2]; pnpm issue discussing the pnpm9-era ignore-dep-scripts behavior vs v10 allowlisting [3].
🌐 Web query:
pnpm v9.0 workspace configuration keys settings
💡 Result:
pnpm v9.0 workspace config: where keys live
pnpm-workspace.yaml (defines the workspace + package globs)
Top-level keys you’ll commonly use:
packages: glob patterns to include/exclude workspace packages (supports negation with!). [1]catalog,catalogs: dependency “catalog” definitions (if you use catalogs). [1]
Workspace .npmrc (workspace behavior/settings keys)
Workspace-related keys (as of pnpm 9.x docs):
link-workspace-packages(true | false | deep) [2]prefer-workspace-packages(boolean) [2]shared-workspace-lockfile(boolean) [2]save-workspace-protocol(true | false | rolling) [2]include-workspace-root(boolean) [2]ignore-workspace-cycles(boolean) [2]disallow-workspace-cycles(boolean) [2]
pnpm v9.0.0 default changes that affect workspaces
link-workspace-packagesis disabled by default (workspace packages are linked by default only when you use theworkspace:protocol). [3]hoist-workspace-packagesis enabled by default. [3]enable-pre-post-scriptsis enabled by default. [3]dedupe-injected-depsis enabled by default. [3]
Sources: [1] [2] [3]
🏁 Script executed:
# Check the .npmrc to see if those keys are configured there
cat ./.npmrcRepository: THM-Health/PILOS
Length of output: 73
🏁 Script executed:
# Double-check the exact pnpm version being used
cat ./package.json | head -50 | grep -E "(packageManager|pnpm)"Repository: THM-Health/PILOS
Length of output: 207
🏁 Script executed:
# Also check if there are any docs or comments about these settings
rg -A 2 -B 2 "ignoredBuiltDependencies|onlyBuiltDependencies|sideEffectsCache" --type yamlRepository: THM-Health/PILOS
Length of output: 382
Critical: Invalid pnpm v9 configuration and indentation inconsistency.
Line 11 uses 1-space indent for the 'docs' entry, inconsistent with the 2-space indentation used elsewhere in the file.
More importantly, ignoredBuiltDependencies and onlyBuiltDependencies (lines 1–6) are not valid in pnpm v9 (your project uses pnpm v9.0 per pnpm-lock.yaml). These keys were introduced in pnpm v10.1.0 and v10.19.0 respectively and will be silently ignored. Either upgrade to pnpm v10+ or remove these unsupported keys. The sideEffectsCache: false setting is valid for v9 but unusual—verify this is intentional.
🤖 Prompt for AI Agents
In `@pnpm-workspace.yaml` around lines 1 - 11, The pnpm config contains
unsupported v10+ keys and an inconsistent indent: remove or upgrade—either
delete the keys ignoredBuiltDependencies and onlyBuiltDependencies (they're
invalid in pnpm v9) or upgrade the project to pnpm v10+ so those keys are
honored; also fix the packages list indentation so the 'docs' entry uses
two-space indentation like the rest (ensure packages: followed by " - 'docs'"
uses consistent spacing); finally confirm sideEffectsCache: false is
intentional. Use the exact key names ignoredBuiltDependencies,
onlyBuiltDependencies, sideEffectsCache, and packages to locate the entries.
Fixes #
Type
Checklist
Changes
Other information
Summary by CodeRabbit