migrate into pnpm #560
migrate into pnpm #560
Conversation
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 5 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 24 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| WORKDIR /app | ||
|
|
||
| # Copy package files | ||
| COPY package.json package-lock.json* ./ |
There was a problem hiding this comment.
The Dockerfile copies package-lock.json* but should copy pnpm-lock.yaml instead for pnpm. After migrating to pnpm, the lockfile has changed from package-lock.json to pnpm-lock.yaml.
| COPY package.json package-lock.json* ./ | |
| COPY package.json pnpm-lock.yaml ./ |
| # Copy package files | ||
| COPY package.json package-lock.json* ./ |
There was a problem hiding this comment.
The pnpm-workspace.yaml configuration file should be copied to the Docker image. Pnpm requires this workspace configuration file to properly install dependencies in a monorepo setup.
| # Copy package files | |
| COPY package.json package-lock.json* ./ | |
| # Copy package files (including pnpm workspace and lockfile) | |
| COPY package.json pnpm-workspace.yaml pnpm-lock.yaml* package-lock.json* ./ |
| "dev": "tsx watch src/index.ts", | ||
| "start": "node dist/index.js", | ||
| "prepublishOnly": "npm run build" | ||
| "prepublishOnly": "npm i -g pnpm && pnpm run build" |
There was a problem hiding this comment.
Installing pnpm globally during the prepublishOnly phase could fail if npm is not available in the publishing environment, or if there are permission issues. Consider using npx to run pnpm directly, or ensure pnpm is pre-installed in the publishing environment. A more robust approach would be to use corepack enable if available, or to handle the pnpm installation separately.
| "prepublishOnly": "npm i -g pnpm && pnpm run build" | |
| "prepublishOnly": "corepack enable && pnpm run build" |
| onlyBuiltDependencies: | ||
| - electron | ||
| - electron-winstaller | ||
| - esbuild | ||
| - protobufjs | ||
| - sharp | ||
| - unrs-resolver | ||
| - workerd |
There was a problem hiding this comment.
The pnpm-workspace.yaml file is missing the required packages: field to define workspace packages. Since this project has packages in the packages/ directory (claude-plugin and mcp-server), the workspace configuration should include a packages: array. The onlyBuiltDependencies option is also not a standard top-level pnpm configuration - this should be onlyBuiltDependencies under pnpm in package.json, or the configuration needs to follow the correct pnpm workspace format.
| onlyBuiltDependencies: | |
| - electron | |
| - electron-winstaller | |
| - esbuild | |
| - protobufjs | |
| - sharp | |
| - unrs-resolver | |
| - workerd | |
| packages: | |
| - "packages/*" |
| # Install pnpm using npm | ||
| RUN npm install -g pnpm |
There was a problem hiding this comment.
npm install -g pnpm pulls and executes an unpinned third-party build tool at image build time, so any compromise of the pnpm package or the npm registry could inject malicious code into your build with access to the build environment. To reduce supply-chain risk and ensure reproducible builds, pin pnpm to a specific version or use a built-in package manager mechanism (for example via packageManager/corepack) that gives you a fixed, integrity-checked toolchain.
| # Install pnpm using npm | ||
| RUN npm install -g pnpm |
There was a problem hiding this comment.
This npm install -g pnpm in the builder stage again installs and executes an unpinned third-party tool at build time, so a compromised pnpm release or npm registry incident could run attacker-controlled code with access to your build arguments and environment variables (including any secrets passed via ARG/ENV). Prefer pinning pnpm to a specific version or using a deterministic package-manager bootstrap (e.g., corepack with packageManager) so the build toolchain is stable and less susceptible to supply-chain compromise.
1 participant
closes #551